Repository navigation
fix: stop write transactions from failing on windows - #39
Merged
Merged
Conversation
CI has been broken since the GraphDB 12 upgrade. GraphDB 12 depends on ai.graphwise:license-common, which you can't download without Ontotext credentials, and the 12.0-TR3 build needs a licence to run anyway. Add a public-ci profile that tests against GraphDB 10.8.14 instead, since it runs without a licence, and mention it in the README for anyone building without credentials. It needs Trove 2 on the test classpath, and the tests now cope with RDF4J 4 (the two triple-term tests skip there). The packaging smoke image moves to 10.8.14 and Java 25 as well. The default build still targets GraphDB 12. CI just can't test it until we have credentials. While here, update the README for GraphDB 12 and JDK 25.
Every write transaction on Windows was failing with "Unable to persist the GeoSPARQL transaction marker". We sync the marker's directory by opening it as a FileChannel, and Windows doesn't let you do that, so it throws AccessDeniedException. Skip the directory sync on Windows, same as Lucene does. File contents still get fsynced, and other platforms still fail if a directory sync fails, since recovery depends on the marker actually being on disk.
Run the full suite and the plugin archive check on windows-latest so this kind of thing gets caught next time. The Docker smoke test stays on Linux. The doc contract test also needed to normalise line endings, since Git checks the README out with CRLF on Windows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes #38. On Windows, any write transaction that changes geometries fails with HTTP 500, "Unable to persist the GeoSPARQL transaction marker". Since 4.0.0,
DurableFileOperationssyncs a file's parent directory by opening the directory as aFileChannel, and Java can't do that on Windows: it throwsAccessDeniedException. 3.1.1 never synced directories, which is why it worked.The fix
forceDirectorynow returns early on Windows when the directory exists. Everything else stays as it was: file contents are still forced to disk, configuration replacement is still an atomic move, and the marker is still written before the Lucene commit. Nothing changes on Linux or macOS.On Windows, a GraphDB crash is still fully covered: the marker is still there at restart, and recovery works as before. A marker created or deleted just before an OS crash or power cut might not survive, though, because the directory entry isn't flushed. Lucene skips directory syncs on Windows too, so the index files were already in the same position.
Why not Lucene's approach
The issue suggests following Lucene's
IOUtils.fsync, which also ignores failed directory syncs on other systems. I kept those failures. At startup, the marker is how the plugin finds out that a transaction's outcome is unknown and the index needs rebuilding, so quietly skipping a failed sync on Linux or macOS would weaken the one thing it's there for.Tests
DurableFileOperationsTestnow writes, replaces and deletes real files, so it fails on Windows without the fix.windowsDirectoryChannelsRejectReadAccessonly runs on Windows and checks that the JDK still rejects directory channels.CI
CI hasn't been able to build since the GraphDB 12 upgrade. GraphDB 12.0.0-Trove4J-RC1 depends on
ai.graphwise:license-common, which Ontotext's Maven repository only serves to authenticated users, and the tests need a GraphDB licence to run.So I added a
public-ciprofile that builds and tests against GraphDB 10.8.14, which needs neither, and a Windows job that runs the suite with it. The packaging smoke test moves to GraphDB 10.8.14 and Java 25 too, with Trove 3 added alongside GraphDB 10's Trove 2. The default build still targets GraphDB 12, and the release pipeline already skips tests because it has no licence, so this adds a test signal rather than replacing one.The catch is that while public CI uses this profile, main and test sources have to compile against both RDF4J 4 and RDF4J 6, and the two triple-term tests skip on GraphDB 10. If public CI could get the GraphDB 12 artifacts and a test licence, the profile could go.
None of this has run inside GraphDB 12, since that needs a licence. The fix itself doesn't touch any GraphDB or RDF4J API.
While there, I updated the README for GraphDB 12 and JDK 25, and added a note on building with
public-ciwhen you don't have Ontotext credentials.