Skip to content

Conversation

@Aklakan
Copy link
Contributor

@Aklakan Aklakan commented Sep 5, 2025

GitHub issue resolved (no issue created yet).

Pull request Description: Upgrade to kryo5. Need yet to test whether spatial indexes created with kryo4 can be loaded with kryo5. Also, there was an inconsistent use of ConcurrentHashMap and LinkedHashMap that was revealed when a test case failed with kryo5.
In my first experiments I was not able to load a spatial index created with kryo4 using kryo5.


  • [ ] Tests are included.
  • Commits have been squashed to remove intermediate development commit messages.
  • Key commit messages start with the issue number (GH-xxxx)

By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.


See the Apache Jena "Contributing" guide.

@Aklakan Aklakan marked this pull request as draft September 5, 2025 20:11
@Aklakan Aklakan force-pushed the 20250905_kryo5 branch 3 times, most recently from c92a786 to 78d19a2 Compare September 10, 2025 15:14
@Aklakan
Copy link
Contributor Author

Aklakan commented Sep 10, 2025

I was not able to load a spatial-index created with kryo4 using kryo5 - it fails to deserialize the ConcurrentHashMap whose registration was missing. Kryo4 auto-registered classes in that case, whereas kryo5 by default requires registration. There is a flag kryo5.setRegistrationRequired(false); but this does not make loading work.

The question is whether to schedule a kryo5 upgrade for Jena6.

@afs
Copy link
Member

afs commented Sep 11, 2025

The question is whether to schedule a kryo5 upgrade for Jena6.

Probably, yes. It is safer.

A major version change to Jena comes every 2 years when the supported java version changes so the opportunity is now.

Running both, while possible, is a maintenance cost.

Even if Kryo4 works now, bug fixes and security fixes will likely be to kryo5 first. I don't see much development around kryo4.

Choosing Kryo5 does not impact applications using Kryo4 elsewhere in their codebase.

@afs afs added the Jena6 Changes relating to Jena6 label Oct 5, 2025
@afs
Copy link
Member

afs commented Oct 27, 2025

No rush - trying to get all the "bumps" done early in the 6. cycle,; this one is isolated so not getting in the way of anything outside of geosparql - can this merged now or are there other changes necessary?

@Aklakan Aklakan marked this pull request as ready for review October 27, 2025 14:32
@Aklakan Aklakan marked this pull request as draft October 27, 2025 14:33
@Aklakan
Copy link
Contributor Author

Aklakan commented Oct 27, 2025

There are a couple of small QoL improvements that I should add to avoid confusion.

I think it is still possible to read the metadata (from kryo's perspective a plain string) of spatial indexes created with kryo4 using kryo5.

  • If so, then i can add a check whether the version field is 2.0.0 and raise an exception that this format is no longer supported. Deleting the spatial index file should re-create it on Fuseki start up.
  • I should use a different version value for kyro5 in the metadata, such as 3.0.0.

@Aklakan
Copy link
Contributor Author

Aklakan commented Oct 29, 2025

I added the version check - with an old index it now outputs:

org.apache.jena.geosparql.spatial.SpatialIndexException: Spatial index version 2.0.0 is no longer supported.
Move or delete this file to allow for creation of a new index in its place: /foo/bar/spatial.index
	at org.apache.jena.geosparql.spatial.index.v2.SpatialIndexIoKryo.load(SpatialIndexIoKryo.java:219)

@Aklakan Aklakan marked this pull request as ready for review October 29, 2025 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Jena6 Changes relating to Jena6

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants