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

Backups Fail on Docker Desktop for Windows #48

Open
nakanotti opened this issue Apr 25, 2022 · 23 comments
Open

Backups Fail on Docker Desktop for Windows #48

nakanotti opened this issue Apr 25, 2022 · 23 comments
Assignees
Labels
bug Something isn't working

Comments

@nakanotti
Copy link

Describe the bug or issue being faced
I had an error occured every backup time.
ex)

[error   ] Unable to access world path at '/backups/world.20220425-141732.mcworld'

Why throw exception always here?
<Sources/Bedrockifier/Model/World.swift>

public struct World {
    public enum WorldType {
        case folder
        case mcworld
        case javaBackup

        init(url: URL) throws {
            let isDirectory = try WorldType.isDirectory(url: url)

The url is '/backups/world.20220425-141732.mcworld'.
These backup files will seem to successful...
But, throw exception "not a directory" to failure process.

Configuration and Verbose Logs
config.yml:

containers:
  bedrock:
    - name: aaa
      worlds:
        - /data/worlds/world

schedule:
  daily: 04:00
  startupDelay: 2m
  onPlayerLogin: true
  minInterval: 2m
trim:
  trimDays: 2
  keepDays: 14
  minKeep: 14
loggingLevel: trace

Additional context
Log:

aaa-backup-1  | [14:17:32.982][info    ] Starting Backup of worlds for: aaa (ContainerConnection.swift:124)
aaa-backup-1  | [14:17:32.982][trace   ] Data Received on file descriptor (3) (Terminal.swift:296)
aaa-backup-1  | [14:17:32.982][trace   ] Processing Listener (Terminal.swift:318)
aaa-backup-1  | [14:17:32.983][trace   ] Data Received on file descriptor (3) (Terminal.swift:296)
aaa-backup-1  | [14:17:32.983][trace   ] Processing Listener (Terminal.swift:318)
aaa-backup-1  | [14:17:32.986][info    ] Backing Up: world (ContainerConnection.swift:130)
aaa-backup-1  | [14:17:46.551][error   ] Unable to access world path at '/backups/world.20220425-141732.mcworld' (ContainerConnection.swift:134)
aaa-backup-1  | [14:17:46.551][error   ] Backup of world at /data/worlds/world failed. (ContainerConnection.swift:135)
aaa-backup-1  | [14:17:46.551][error   ] Backups for aaa had failures... (ContainerConnection.swift:155)

Thank you for your help.

@nakanotti nakanotti added the bug Something isn't working label Apr 25, 2022
@Kaiede
Copy link
Owner

Kaiede commented Apr 25, 2022

This looks like there might be some permissions related error at play here, where the file is created and written to, but cannot be read from. It's not throwing because the file isn't a directory, but because it can't determine if it is a directory. i.e. something's blocking reading the file's attributes.

Can you share your docker-compose.yml, and ls -l on the backup files (the mcworld files) that are being created on the host so we can see what user is winding up owning them, and how the service is configured? Also, parts of the verbose log from the start would be helpful here as well, as it would give an idea of what permissions the service is running with.

@nakanotti
Copy link
Author

I also first suspected access rights.
I entered the CLI and checked it, but I was able to access it as a file without any problems.

Since the world data is large, I will try to make the minimum configuration that can be confirmed to be reproduced.

root@e8bd8f3c3d5b:/# ls -al /backups
total 585836
drwxrwxrwx 1 root root      4096 Apr 26 13:53 .
drwxr-xr-x 1 root root      4096 Apr 26 13:45 ..
-rwxrwxrwx 1 root root       272 Apr 25 14:16 config.yml
-rw-r--r-- 1 root root 599890209 Apr 26 13:50 world.20220426-134955.mcworld

aaa-backup-1.log

<docker-compose.yml>

version: "3"

services:
  aaa:
    container_name: aaa
    image: itzg/minecraft-bedrock-server
    ports:
      - "19136:19136/udp"
    tty: false
    stdin_open: true
    restart: on-failure:1
    volumes:
      - "/var/run/docker.sock:/var/run/docker.sock"
      - "./data:/data"
    environment:
      TZ: "Asia/Tokyo"
      LANG: "ja_JP.UTF-8"
      LANGUAGE: "ja_JP"
      SERVER_NAME: "AAA World"
      SERVER_PORT: "19136"
      EULA: "TRUE"
      LEVEL_TYPE: "default"
      LEVEL_NAME: "world"
      GAMEMODE: "survival"
      DIFFICULTY: "hard"
      ALLOW_CHEATS: "true"
      MAX_PLAYERS: "20"
      ONLINE_MODE: "true"
      ALLOW_LIST: "false"
      DEFAULT_PLAYER_PERMISSION_LEVEL: "visitor"
    labels:
      - manymine.enable=true
    networks:
      - proxy_default

  backup:
    image: kaiede/minecraft-bedrock-backup
    restart: on-failure:1
    depends_on:
      - "aaa"
    environment:
      TZ: "Asia/Tokyo"
      LANG: "ja_JP.UTF-8"
      LANGUAGE: "ja_JP"
    tty: true
    volumes:
      - "/var/run/docker.sock:/var/run/docker.sock"
      - "./backups:/backups"
      - "./data:/data"
    networks:
      - proxy_default

networks:
  proxy_default:
    external: true

@Kaiede
Copy link
Owner

Kaiede commented Apr 26, 2022

I can also make changes so that the underlying error is more completely reported, and push that out to the test tag which will give us more details and help diagnose similar issues in the future. I’ll do that tomorrow.

And yes, it does look like the backup is being made okay. But somehow the file then can’t be checked to confirm we got a valid backup afterwards. Strange. It’s possible that something about the size of the backup is somehow triggering a bug in Swift on Linux that will need to be worked around.

@nakanotti
Copy link
Author

I found successful case...

<docker-compose.yml>

  volumes:
#   - "./backups:/backups" ... throw exception in "Docker Desktop for Windows".
      - "aaa-backups:/backups" # using docker volumes

volumes:
  aaa-backups:

It may be a problem due to the permissions of Windows local folder!

@nakanotti
Copy link
Author

I am not understand swift.
If possible, how about outputting detailed information as follows?
<World.swift>

        private static func isDirectory(url: URL) throws -> Bool {
            do {
                let values = try url.resourceValues(forKeys: [.isDirectoryKey])
                return values.isDirectory == true
            } catch {
+		Library.log.trace(error.localizedDescription)
                throw WorldError.invalidUrl(url: url)
            }
        }

@Kaiede
Copy link
Owner

Kaiede commented Apr 26, 2022

It may be a problem due to the permissions of Windows local folder!

Very possibly. I don't have any experience with Docker Desktop on Windows. I wonder if it uses WSL, and if it doesn't, if Docker in WSL behaves any better.

If possible, how about outputting detailed information as follows?

That was close to my intent yes, but it was close to midnight local time, so I wasn't going to jump into it until I had gotten some sleep. :)

I've made the change, and merged it into main, so it will be available on the test tag once this build run completes: https://github.com/Kaiede/Bedrockifier/actions/runs/2227958575

@nakanotti nakanotti changed the title Exception occured always on backup time. Exception occured always at backup time on Docker Desktop for Windows. Apr 27, 2022
@nakanotti
Copy link
Author

aaa-backup-1-20220427.log

[error detail]

[16:44:25.995][error   ] Unable to access world path at '/backups/world.20220427-164425.mcworld': Error Domain=NSCocoaErrorDomain Code=260 "The file doesn’t exist." (ContainerConnection.swift:134)

[console in aaa-backup-1]

root@829f2da6288d:/opt/bedrock# ls -al /backups
total 184
drwxrwxrwx 1 root root   4096 Apr 27 16:44 .
drwxr-xr-x 1 root root   4096 Apr 27 16:43 ..
-rwxrwxrwx 1 root root    269 Apr 27 16:43 config.yml
-rw-r--r-- 1 root root 183610 Apr 27 16:44 world.20220427-164425.mcworld

@Kaiede
Copy link
Owner

Kaiede commented Apr 27, 2022

Yeah, this clearly suggests something is weird with fetching attributes from files in this specific scenario. What’s not clear is if this is a problem in Apple’s Foundation library, WSL or Docker. Because you mentioned named containers work, I’ve been trying to research what the difference is between named volumes and bound mount points on Windows, but haven’t had much luck so far.

Not being able to read the file attributes is going to be messy as it’s key to how backup trimming works, and it’s not clear at the moment how to work around this issue. So unfortunately, I can’t say there will be a quick resolution. But I do want to keep this open so I can track this issue and update the title to make it easier for other folks to see this information.

@Kaiede Kaiede changed the title Exception occured always at backup time on Docker Desktop for Windows. Backups Fail on Docker Desktop for Windows Apr 27, 2022
@Kaiede Kaiede self-assigned this Apr 27, 2022
@Kaiede Kaiede pinned this issue Apr 27, 2022
@Kaiede
Copy link
Owner

Kaiede commented Apr 27, 2022

One last thing I want to confirm, if you would. What is the location of your docker-compose.yml?

Is it entirely in the Linux filesystem in WSL? Or is it hosted on a Windows drive?

@nakanotti
Copy link
Author

It hosted on a Windows drive.
ex)
C:\MCS\docker\aaa\docker-compose.yml

  • Docker Desktop for Windows version 4.7.1 (77678)
  • aaa.zip

start up command:

C:\MCS\docker\aaa>docker compose up -d
[+] Running 3/3
 - Network aaa_default     Created                                                                                 0.0s
 - Container aaa           Started                                                                                 2.0s
 - Container aaa-backup-1  Started                                                                                 2.3s

stop down command:

C:\MCS\docker\aaa>docker compose down
[+] Running 3/3
 - Container aaa-backup-1  Removed                                                                                10.4s
 - Container aaa           Removed                                                                                 0.7s
 - Network aaa_default     Removed                                                                                 0.2s

@Kaiede
Copy link
Owner

Kaiede commented Apr 28, 2022

Yeah, I wonder if you’ve uncovered an issue when bind-mounting to the Windows filesystem which has a couple known limitations. Docker themselves recommend bind-mounting to the Linux filesystem in WSL instead: https://docs.docker.com/desktop/windows/wsl/. There might also be some differences depending on if your setup using WSL 1 or 2 as well. Do you happen to know which version of WSL is being used?

Since named volumes are part of the Linux filesystem, I suspect the recommendation of bind-mounting to the Linux filesystem will also work. Could you try it?

@nakanotti
Copy link
Author

nakanotti commented Apr 28, 2022

Do you happen to know which version of WSL is being used?

I'm using WSL 2.

Could you try it?

Yes. I installed Ubuntu with Docker on WSL2.
It was successful! (It was using Docker Engine of Docker Desktop.)

[Ubuntu shell on Linux filesystem]

/opt/docker/aaa$ docker compose up -d
[+] Running 3/3
 ⠿ Network aaa_default     Created                                                                                 0.0s
 ⠿ Container aaa           Started                                                                                 0.6s
 ⠿ Container aaa-backup-1  Started                                                                                 1.0s

But, In the following case has other trouble occurred.

[PowerShell on Linux volume via windows filesystem]

PS Microsoft.PowerShell.Core\FileSystem::\\wsl$\Ubuntu\opt\docker\aaa> docker compose up -d
[+] Running 3/3
 - Network aaa_default     Created                                                                                 0.0s
 - Container aaa           Started                                                                                 0.8s
 - Container aaa-backup-1  Started                                                                                 1.1s

[log]

DEBU[0000] Using /backups to match uid and gid          
DEBU[0000] Resolved UID=1000 from match path            
DEBU[0000] Resolved GID=999 from match path             
[17:41:19.907][info    ] Initializing Bedrockifier Daemon
[17:41:19.908][info    ] Loading Configuration From: /backups/config.yml
[17:41:19.910][info    ] Configuration Loaded, Entering Event Loop... (main.swift:84)
[17:41:19.919][info    ] Host PTY Handle Opened: /dev/pts/1 (6) (Terminal.swift:141)
[17:41:19.920][info    ] Child PTY Handle Opened: /dev/pts/1 (7) (Terminal.swift:162)
[17:41:19.920][debug   ] Process attached (Terminal.swift:81)
[17:41:19.920][debug   ] Starting Container Process (ContainerConnection.swift:69)
[17:41:19.921][info    ] Checking for servers that might not be cleaned up (BackupActor.swift:73)
[17:41:19.921][info    ] Backup Time: 04:00 (Service.swift:162)
[17:41:19.921][info    ] Next Backup: Apr 29, 2022, 4:00:00 AM (Service.swift:169)
[17:41:19.921][debug   ] [interval] Scheduled For Apr 29, 2022, 4:00:00 AM (ServiceTimer.swift:73)
[17:41:19.921][info    ] Starting Listeners for Containers (Service.swift:188)
[17:41:19.921][debug   ] [interval] Registered (ServiceTimer.swift:102)
[17:41:19.921][info    ] Backup Minimum Interval is 120.0 seconds (Service.swift:92)
[17:41:19.939][trace   ] Data Received on file descriptor (6) (Terminal.swift:296)
[17:41:19.939][trace   ] Processing Listener (Terminal.swift:318)
[17:41:19.940][trace   ] Data Received on file descriptor (6) (Terminal.swift:296)
[17:41:19.940][trace   ] Processing Listener (Terminal.swift:318)
[17:41:19.942][trace   ] Process terminated (Process.swift:41)
[17:41:19.942][debug   ] Calling process detach handlers (Terminal.swift:94)
[17:41:19.942][debug   ] Process detached (Terminal.swift:99)

Process terminated...

I will switch to WSL + docker soon... (T-T)

@farmertip
Copy link

Hello @Kaiede, I'm not much of a programmer but I think I've made some headway resolving this. I too ran into the same problem that @nakanotti ran into. First, with respect to this code in world.swift (Line 36):

   init(url: URL) throws {
        let isDirectory = try WorldType.isDirectory(url: url)
        if isDirectory {
            self = .folder
        } else if url.pathExtension == "mcworld" {
            self = .mcworld
        } else if url.pathExtension == "zip" {
            self = .javaBackup
        } else {
            throw WorldError.invalidWorldType
        }
    }

I changed it to:

    init(url: URL) throws {
        if url.pathExtension == "mcworld" {
            self = .mcworld
        } else if url.pathExtension == "zip" {
            self = .javaBackup
        } else {
            let isDirectory = try WorldType.isDirectory(url: url)
            if isDirectory {
                self = .folder
            } else { 
                throw WorldError.invalidWorldType
            }
        }
    }

This passed the initial hangup that @nakanotti mentioned. Specifically, the url.resourceValues(forKeys: [.isDirectoryKey] function in line 50 always threw an error (because the URL passed was a file, not a folder). No idea why but this always problematic.

    private static func isDirectory(url: URL) throws -> Bool {
        do {
            let values = try url.resourceValues(forKeys: [.isDirectoryKey])
            return values.isDirectory == true
        } catch {
            throw WorldError.invalidUrl(url: url, innerError: error)
        }
    }

However I encountered another problem after that. I narrowed it down to an issue with the .part file extension. I just disabled that temporary by commenting out lines 147 and 160 respectively:
// let tempUrl = url.appendingPathExtension(World.partialPackExt)
// try FileManager.default.moveItem(at: tempUrl, to: url)

After that, everything seemed to begin working correctly... or at least it no longer gave me errors. I haven't determined if the .part file extension was somehow related to the first problem (maybe there needs to be a case for .part in the init?). However, this suggests that the overall problem is not likely related to a Docker Desktop WSL/WSL2, Windows, or a permissions problem. It feels more like a race condition and/or a problem with the .part file extension not being handled properly. Once again, I'm not much of a programmer (never touched Swift until now) but I'm pretty good at mashing keys and doing Google searches. This is my first time posting on GitHub. I guess I really, really wanted your backup system to work for me. LOL. I hope this can provide some guidance on where to go next.

BTW, now that I have this working, wow this is awesome. I absolutely love having my Bedrock worlds backup after exiting my server. Way better than crappy batch file scripts I put together for daily backups. Thanks a ton!!!

@Kaiede
Copy link
Owner

Kaiede commented Aug 8, 2022

Specifically, the url.resourceValues(forKeys: [.isDirectoryKey] function in line 50 always threw an error (because the URL passed was a file, not a folder). No idea why but this always problematic.

That's a bug in the Swift Foundation library, or WSL. It shouldn't be throwing, but instead just set the key to false like Linux and MacOS does.

By moving it, it does work around the issue, but it also means that if I have a folder I've called "foo.zip" then the logic breaks in new ways as it tries to unzip a folder. Fun. But this could make it easier to look up the issue and see if there might be fixes in newer versions of Swift, or if it is fixed in the upcoming 5.7 release.

Do you have the error from the part file issue? It sounds a lot like WSL doesn't like the move of the temporary file to the final location, but the error would be valuable in understanding why.

@farmertip
Copy link

Hi @Kaiede - I have messed with the code a bit more to resolve your file/folder concern. So far, this seems to work:

    init(url: URL) throws {
        let isFile = try WorldType.isFile(url: url)
        if isFile {
            if url.pathExtension == "mcworld" {
                self = .mcworld
            } else if url.pathExtension == "zip" {
                self = .javaBackup
            } else {
                // Not a mcworld or zip file...
                throw WorldError.invalidWorldType
            }

        } else {
            let isDirectory = try WorldType.isDirectory(url: url)
            if isDirectory {
                self = .folder
            } else {
                // What in the heck am I??
                throw WorldError.invalidWorldType
            }
        }  
    }
    private static func isFile(url:URL) throws -> Bool {
        do {
            let values = try url.resourceValues(forKeys: [.isRegularFileKey])
            return values.isDirectory == false
        } catch {
            Library.log.info("isFile resourceValues problem")
            throw WorldError.invalidUrl(url: url, innerError: error)
        }
    }

    private static func isDirectory(url: URL) throws -> Bool {
        do {
            let values = try url.resourceValues(forKeys: [.isDirectoryKey])
            return values.isDirectory == true
        } catch {
            throw WorldError.invalidUrl(url: url, innerError: error)
        }
    }

With respect to errors after this change, I get the following (after I log out of my server):

MinecraftTEST     | [2022-08-11 15:39:06:935 INFO] Player connected: FarmerTip, xuid: XXXXXXXXXXXXXXXX
BackupSystemTest  | [11:39:06.520][info    ] Player Logged In: MinecraftTEST, Players Active: 1
BackupSystemTest  | [11:39:19.108][info    ] Player Logged Out: MinecraftTEST, Players Active: 0
BackupSystemTest  | [11:39:19.108][info    ] Running Single Backup for MinecraftTEST
MinecraftTEST     | [2022-08-11 15:39:19:526 INFO] Player disconnected: FarmerTip, xuid: XXXXXXXXXXXXXXXX
MinecraftTEST     | save hold
MinecraftTEST     | Saving...
MinecraftTEST     | save query
MinecraftTEST     | Data saved. Files are now ready to be copied.
MinecraftTEST     | MinecraftTEST/db/000025.log:26818, MinecraftTEST/db/000026.ldb:118504, MinecraftTEST/db/CURRENT:16, MinecraftTEST/db/MANIFEST-000023:355, MinecraftTEST/level.dat:2544, MinecraftTEST/level.dat_old:2544, MinecraftTEST/levelname.txt:13
BackupSystemTest  | [11:39:21.285][info    ] Starting Backup of worlds for: MinecraftTEST
BackupSystemTest  | [11:39:21.297][info    ] Backing Up: MinecraftTEST
BackupSystemTest  | [11:39:21.423][error   ] The operation could not be completed. A file with the same name already exists.
BackupSystemTest  | [11:39:21.423][error   ] Backup of world at /MinecraftTEST/worlds/MinecraftTEST failed.
MinecraftTEST     | save resume
BackupSystemTest  | [11:39:21.423][error   ] Backups for MinecraftTEST had failures...
MinecraftTEST     | Changes to the world are resumed.
BackupSystemTest  | [11:39:21.431][error   ] Single Backup for MinecraftTEST failed
BackupSystemTest  | [11:39:21.432][error   ] Server container had worlds that failed to backup: /MinecraftTEST/worlds/MinecraftTEST

I was digging deeper into this last weekend. I seem to recall removing the .part file extension naming/renaming stuff and it resolved the problem. The good thing is, the file seems to back up fine regardless of the error. I'll try diving deeper in the next few days to pinpoint what specifically is triggering the erroneous error. Thanks!

@farmertip
Copy link

Oh, my attempt to determine if the file/folder is a file might not have worked. I reverted to the code in the first post (without using resourceValue), I get the following:

MinecraftTEST     | [2022-08-11 16:06:46:211 INFO] Player connected: FarmerTip, xuid: XXXXXXXXXXXXXXXX
BackupSystemTest  | [12:06:46.797][info    ] Player Logged In: MinecraftTEST, Players Active: 1
MinecraftTEST     | [2022-08-11 16:07:28:314 INFO] Player disconnected: FarmerTip, xuid: XXXXXXXXXXXXXXXX
BackupSystemTest  | [12:07:28.913][info    ] Player Logged Out: MinecraftTEST, Players Active: 0
BackupSystemTest  | [12:07:28.915][info    ] Running Single Backup for MinecraftTEST
MinecraftTEST     | save hold
MinecraftTEST     | Saving...
MinecraftTEST     | save query
MinecraftTEST     | A previous save has not been completed.
MinecraftTEST     | save query
MinecraftTEST     | Data saved. Files are now ready to be copied.
MinecraftTEST     | MinecraftTEST/db/000035.log:221870, MinecraftTEST/db/000036.ldb:118456, MinecraftTEST/db/CURRENT:16, MinecraftTEST/db/MANIFEST-000033:416, MinecraftTEST/level.dat:2544, MinecraftTEST/level.dat_old:2544, MinecraftTEST/levelname.txt:13
BackupSystemTest  | [12:07:41.010][info    ] Starting Backup of worlds for: MinecraftTEST
BackupSystemTest  | [12:07:41.017][info    ] Backing Up: MinecraftTEST
BackupSystemTest  | [12:07:41.137][error   ] World archive is not a valid zip or mcworld file
BackupSystemTest  | [12:07:41.137][error   ] Backup of world at /MinecraftTEST/worlds/MinecraftTEST failed.
MinecraftTEST     | save resume
BackupSystemTest  | [12:07:41.137][error   ] Backups for MinecraftTEST had failures...
MinecraftTEST     | Changes to the world are resumed.
BackupSystemTest  | [12:07:41.163][error   ] Single Backup for MinecraftTEST failed
BackupSystemTest  | [12:07:41.164][error   ] Server container had worlds that failed to backup: /MinecraftTEST/worlds/MinecraftTEST

Slightly different results. Makes me think that .isRegularFileKey doesn't work either. At least this method pointed to the .part file extension problem I mentioned earlier. Seems clear that the ability to distinguish between a file and folder (or how it's handled in Docker) is the culprit. Maybe there's another way around this...

@Kaiede
Copy link
Owner

Kaiede commented Aug 11, 2022

Hmm, ideally, I'd love to be able to attack this with some form of unit testing, but it requires a pretty specific environment: Linux container being run on Windows, with the files being accessed from the host. So it's more a big integration test that requires setup to perform. Maybe I could setup a Dockerfile specifically to run the test suite from within a docker container, but it'd take some work.

Out of curiosity, how did you setup this environment on Windows? While I don't currently have something setup, I could probably setup a Windows VM to replicate this, and then start playing around with possible solutions using a swift development docker container. So long as the POSIX APIs work correctly, it should be possible to drop down to the C APIs and call them directly to check for file vs directory. At least as long as the Zip framework doesn't have issues of its own.

@Kaiede Kaiede reopened this Aug 11, 2022
@farmertip
Copy link

Well, my setup isn't probably the most efficient by any means. I've been running your build-docker.sh script on my mid-2015 Macbook Pro to build the image. From there I tag it and push to docker hub. Then I hop over to my Windows box where I've got a quick-and-dirty docker-compose configuration which spins up the minecraft-bedrock-backup (obviously pulling the latest version of my own hackery) and a single instance of the itzg/minecraft-bedrock-server. Lastly, I jump into the Minecraft server long enough to trigger the backup event and begin troubleshooting. I know, not great but it's working. The Windows box is where my servers will ultimately reside once this is resolved.

@farmertip
Copy link

Hi @Kaiede - I think I've got a decent solve for this. In World.swift (starting at line 31), the following change resolved the problem with distinguishing between a folder and file on my system:

    public enum WorldType {
        case folder
        case mcworld
        case javaBackup

        init(url: URL) throws {
            // let isDirectory = try WorldType.isDirectory(url: url)

            // if isDirectory {
            if url.hasDirectoryPath {
                self = .folder
            } else if url.pathExtension == "mcworld" {
                self = .mcworld
            } else if url.pathExtension == "zip" {
                self = .javaBackup
            } else {
                throw WorldError.invalidWorldType
            }
        }

        // private static func isDirectory(url: URL) throws -> Bool {
        //     do {
        //         let values = try url.resourceValues(forKeys: [.isDirectoryKey])
        //         return values.isDirectory == true
        //     } catch {
        //         throw WorldError.invalidUrl(url: url, innerError: error)
        //     }
        // }
    }

With that resolved, I also found a work-around with the ".part" file problem. For some reason FileManager.default.moveItem doesn't play nice. Maybe the newly renamed file isn't ready to be opened right after it was renamed. Regardless, in World.swift around line 160 I did the following which solved that problem:

        // With it packed successfully, rename it.
        // try FileManager.default.moveItem(at: tempUrl, to: url)
        try FileManager.default.copyItem(at: tempUrl, to: url)
        try FileManager.default.removeItem(at: tempUrl)

So with those two changes, everything appears to be working correctly on my setup. I haven't had a chance to see if there's errors during the trim process yet. I'll have to figure out a way to simulate that. For now, I'm off to look at my eyelids.

MinecraftTEST     | [2022-08-14 05:59:00:862 INFO] Player connected: FarmerTip, xuid: XXXXXXXXXXXXXXXX
BackupSystemTest  | [01:59:00.943][info    ] Player Logged In: MinecraftTEST, Players Active: 1
MinecraftTEST     | [2022-08-14 05:59:02:728 INFO] Player disconnected: FarmerTip, xuid: XXXXXXXXXXXXXXXX
BackupSystemTest  | [01:59:02.851][info    ] Player Logged Out: MinecraftTEST, Players Active: 0
BackupSystemTest  | [01:59:02.851][info    ] Running Single Backup for MinecraftTEST
MinecraftTEST     | save hold
MinecraftTEST     | Saving...
MinecraftTEST     | save query
MinecraftTEST     | Data saved. Files are now ready to be copied.
MinecraftTEST     | MinecraftTEST/db/000103.log:0, MinecraftTEST/db/000104.ldb:146090, MinecraftTEST/db/CURRENT:16, MinecraftTEST/db/MANIFEST-000101:250, MinecraftTEST/level.dat:2544, MinecraftTEST/level.dat_old:2544, MinecraftTEST/levelname.txt:13
BackupSystemTest  | [01:59:04.487][info    ] Starting Backup of worlds for: MinecraftTEST
BackupSystemTest  | [01:59:04.492][info    ] Backing Up: MinecraftTEST
BackupSystemTest  | [01:59:04.839][info    ] Backed up as: MinecraftTEST.20220814-015904.mcworld
BackupSystemTest  | [01:59:04.839][info    ] Backups for MinecraftTEST finished successfully...
MinecraftTEST     | save resume
MinecraftTEST     | Changes to the world are resumed.
BackupSystemTest  | [01:59:04.914][info    ] Performing Trim Jobs
BackupSystemTest  | [01:59:04.947][info    ] Single Backup Completed

@Kaiede
Copy link
Owner

Kaiede commented Aug 22, 2022

I guess my question was more: Are you just using Docker Desktop on Windows or something else?

With that resolved, I also found a work-around with the ".part" file problem. For some reason FileManager.default.moveItem doesn't play nice. Maybe the newly renamed file isn't ready to be opened right after it was renamed.

Aha, this might explain the issue. Windows tends to be a bit more picky about when you can manipulate a file on disk, and will usually lock it from being renamed so long as it remains open. Something I just re-learned the last time I was playing with Windows a few days ago.

Because the Archive is not closed until the object deallocates, then the file cannot be moved until that happens. Linux/BSD/Mac will let you rename the file while it is still open. So the full fix here is to ensure the Archive is deallocated before we rename. So let's see if that works before falling back to copies, as a copy will be more expensive in time and disk space, especially on Java.

As for the URL fix, that does generally work so long as the URLs properly act the same way all the time. hasDirectoryPath is really only checking for the presence of a trailing slash, which in this case may be working, but could cause problems down the road. I still suspect I'll need to implement something at the POSIX layer.

But thanks for digging into this so far, it has been a huge help narrowing things down while I've been distracted by real life and a separate (large) side project.

@farmertip
Copy link

Yes, just running docker desktop for windows.
Same here, life has been nuts (dealing with a family member's estate, my truck got totaled by a distracted driver, multiple work projects and deadlines, daughter starting kindergarten soon, etc.). I'm looking forward to August being done but September is shaping up to be just as crazy.
For the short-term, this has dropped in my priority but I'm hoping I'll be able to circle back to it in a month or so. Sounds like we both need a well deserved break.

@nakanotti
Copy link
Author

nakanotti commented Aug 29, 2022

Initially, I used the same approach as Mr. @farmertip.
However, the main container "itzg/minecraft-bedrock-server" itself has better performance on linux volume,
so I moved everything to "\\wsl$\Ubuntu\opt\docker\bedrock"...

ex) start up script file from Windows: \\wsl$\Ubuntu\opt\docker\bedrock\up.ps1

wsl -d Ubuntu -- docker compose up -d

We can also add the following message to the error message, as another approach...
If you are using Docker Desktop for Windows, we recommend using a Linux volume.

By the way, I hope that the image size will be smaller.. (^_^;

@Kaiede
Copy link
Owner

Kaiede commented Aug 30, 2022

I've made changes to ensure the Archive is closed before moveItem() is called on the file, and will get it merged as part of this PR: #58

@farmertip This hopefully should fix half of the problem, leaving just the isDirectory concern behind. I still don't yet have a Windows VM with Docker setup, so if you do happen to find some time to take a look, it'd be great. Otherwise I'll take a look once I have that environment available to me.

By the way, I hope that the image size will be smaller.. (^_^;

According to Docker Hub, it looks like a good chunk of the image is actually from installing docker.io bits into the container so I can access the Minecraft container. I could probably remove the docker.io dependency and shave off around 100-120MB if I make folks map in the docker binary and socket file (which then throws Windows compatibility clear out the window, possibly). That would be a breaking change though.

We can also add the following message to the error message, as another approach...
If you are using Docker Desktop for Windows, we recommend using a Linux volume.

I'll call that plan B for now. Since I think with farmertip's help identifying the core areas where things go wrong, they can be fixed and Windows Docker supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants