-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
The "sync" command in backup-loop.sh #189
Comments
I looked later and there is no vanilla sync command https://minecraft.wiki/w/Commands I think the original author really meant the Linux sync command. Now how important that was is a good question. I looked and I must have missed the change during PR and didn't ask. |
I stand corrected. I swear I found something obscure that indicated there was a sync command for Minecraft server and even double checked in ChatGPT (for what that's worth) and it even went so far as to elaborate the difference between "sync" and "save-all flush" commands. But you're right, there is no reference to it and using the command in Minecraft server returns an error. With that said, I agree the original author intended to use the Linux command, but it somehow got wrapped in the rcon-cli retry loop erroneously. However, running the sync command in the mc-backup container would not be effective since any potential data would be resident in the memory of the minecraft-server container. Docker should be handling this at the volume level itself from what I understand since it's intended that data can be shared between services, but if we wanted to be absolutely certain, the command would need to be executed in the server container. Is there a way to facilitate this from the backup container? |
I don't believe Docker would be doing anything special since it's all just Linux inodes mounted into the container/process. Even being called from an adjacent container, it's likely it works as intended since again it's the same filesystem/inode mounted into both containers. ...however, if we're doubting its effectiveness and why it's even there, then probably should just remove the call entirely. |
From what I'm reading, the Docker volume "driver" does consistent writes by default to ensure data is immediately available on the host, and assumedly the other containers accessing named volumes. Nothing to worry about here. However, the OS in the minecraft-server container could potentially have data in cache that has not yet been submitted to storage. Docker cannot do anything about that, and that's where the sync command could help ensure data is truly flushed to the storage layer. If we're erring on the side of caution, perhaps it should be included? |
That'll be for Docker Desktop, right? I was thinking about fully native Docker on Linux on where everything (host and containers) are running in the same kernel against the same filesystems and inodes.
But yes, a point I forgot I was going to make is that it has been working fine for 5 years, so no need to mess with it: ce2ea13#diff-26c954788cf358925d71c7e1ef6dd3824e2884ee29afe73d58091f0775cc5611R286 In conclusion :)
So, just close the issue? |
The following line in backup-loop.sh is missing the "rcon-cli" portion of the command:
retry ${RCON_RETRIES} ${RCON_RETRY_INTERVAL} sync
This results in the command being executed in the mc-backup container instead, calling the Linux "sync" command. There is no need for this command to run inside the mc-backup container and it appears to have been intended to be the
sync
command in Minecraft server, which should follow the save-all command.Suggested fix is to simply add the "rcon-cli" command to this line. Will submit a PR; opening this issue for tracking purposes.
The text was updated successfully, but these errors were encountered: