Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hot-Reloading changed mbtiles #1108

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

candux
Copy link
Contributor

@candux candux commented Dec 31, 2023

This will watch for changes in the mbtiles and hot-reload it.
To keep the implementation simple, the whole renderer is restarted.
This will create a small downtime.

It is very important to replace files atomically.

I tried to test all possible edge-cases, but we (geOps) don't use it in production yet. More testing would be much appreciated.

Signed-off-by: David Weber | geOps <david.weber@geops.com>
GC sometimes cannot do it himself. Maybe a self-referencing problem

Signed-off-by: David Weber | geOps <david.weber@geops.com>
necessary to implement hot mbtiles reloading

Signed-off-by: David Weber | geOps <david.weber@geops.com>
this will watch for changes in the mbtiles and hot-reload it.
To keep the implementation simple, the whole renderer is restartet.
This will create a small downtime.

It is very important to replace files atomically
Signed-off-by: David Weber | geOps <david.weber@geops.com>
@acalcutt
Copy link
Collaborator

acalcutt commented Dec 31, 2023

Nice, I think this will be a great feature. It looks like this is similar to #455 , so I am wondering how you feel this will be affected by the serveAllData, which was a concern in the other thread. I assume the files are just reloaded with the same name, so it wouldn't actually be much of an issue.

The other thing I notice is this focuses on mbtiles, but we now also support pmtiles as a Input File. Can you update this to also support the pmtiles directory and input files that start with pmtiles:\\

In other places in the code, I have changed 'mbtiles' it 'inputFile'.

@candux
Copy link
Contributor Author

candux commented Dec 31, 2023

It looks like this is similar to https://github.com/maptiler/tileserver-gl/pull/455/files , so I am wondering how you feel this will be affected by the serveAllData, which was a concern in the other thread. I assume the files are just reloaded with the same same, so it wouldn't actually be much of an issue.

Yes, I don't think we have any of these problems in this MR. Now that I think of it, most of the concerns raised there should be already solved with this MR. When I started implementing this, I thought that a serveAllData option would be too difficult to get right. But maybe it's not difficult anymore. I'll take another shot at implementing it. Otherwise, I'll update this PR with support for mbtiles

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants