[lweitzendorf]

Jira: https://issues.apache.org/jira/browse/OAK-12486

org.apache.jackrabbit.oak.commons.Buffer was introduced to avoid NoSuchMethodError when Java 9+ compiled code ran on Java 8. Oak now requires Java 17, so that reason no longer applies, while the wrapper's restricted API is tedious to keep up to date and prevents callers from using more concise/efficient ByteBuffer methods.

Changes

• Deprecate Buffer. No other change to oak-commons, so no package version bump there.
• Replace Buffer with ByteBuffer in oak-segment-tar, oak-segment-remote, oak-segment-aws, oak-segment-azure and oak-upgrade. No other module used it.
• This includes the segment persistence SPI and the oak-segment-remote hooks, keeping their method names:
  • SegmentArchiveReader.readSegment / getBinaryReferences, SegmentArchiveWriter.readSegment
  • PersistentCache.readSegment / writeSegment, AbstractPersistentCache.readSegmentInternal
  • AbstractRemoteSegmentArchiveReader.doReadSegmentToBuffer / doReadDataFile, AbstractRemoteSegmentArchiveWriter.doReadArchiveEntry
  • SegmentWriteAction.toBuffer() → toByteBuffer()
• Use ByteBuffer methods where they simplify callers: absolute bulk get, slice(int, int), absolute getInt/getLong, CRC32.update(ByteBuffer), and the existing IOUtils.readFully(FileChannel, int, ByteBuffer). Fewer copies in PersistentRedisCache, S3Directory and Azure SegmentCopy.
• Update AGENTS.md: the minimum Java version is 17 (separate commit).

Breaking changes (internal packages only)

All affected exported packages are @Internal (segment.remote.persistentcache since #3266), so they get major version bumps without a deprecation period:

Package	Version
segment.spi.persistence.persistentcache	7.0.0 → 8.0.0
segment.remote	3.0.0 → 4.0.0
segment.remote.persistentcache	2.1.1 → 3.0.0
segment.remote.queue	1.0.0 → 2.0.0

segment.spi.persistence (SegmentArchiveReader/Writer) is exported unversioned, as on trunk.

Behaviour notes for reviewers

Several reads now use absolute positions and no longer move the caller's buffer position (e.g. BinaryReferencesIndexLoader*, SegmentStream.read).

Testing

Full install with tests and OSGi baseline:

Module	Run	Failed	Skipped
oak-segment-tar	866	0	1
oak-segment-remote	65	0	2
oak-segment-aws	72	0	3
oak-segment-azure	292	0	11
oak-upgrade	184	0	21

RecordCacheTest.weightConvergesAfterAsyncEviction failed once in an earlier local run on an async-eviction timeout; it is untouched by this change and has passed since.

[reschke]

Note that this will make backports to 1.22.x harder (Java 8), but it's hard already. :-)

[reschke]

Unhappy.

This is a public API, and it's a bad sign if we add something here that is for internal use only.

Proposal: leave this class alone, and move the new code into a package below commons.internal - or do not add it at all - given the fact that these are one-liners anyway.

[lweitzendorf]

It would be easy to move wrap, but if I don't have access to the internal buffer field, toByteBuffer needs to copy every time.

[lweitzendorf]

Which we could do, but adding conversion function to and from the native Java equivalent seems justifiable to me, even if they're currently only used internally.

[lweitzendorf]

Alternatively, I could add methods to every API that exposes Buffer and add an alternative that uses ByteBuffer. That would make an expensive conversion less problematic because callers can avoid it entirely by switching to the ByteBuffer version and we can then deprecate the existing functions using Buffer. That would mean we can avoid changing Buffer itself, but we'd then need to bump all the other packages we're adding methods too:

• spi.persistence.persistentcache: 7.0.0 → 7.1.0
• segment.remote: 3.0.0 → 3.1.0
• segment.remote.persistentcache: 2.1.0 → 2.2.0
• segment.remote.queue: 1.0.0 → 1.1.0

[jsedding]

I am not sure that bumping the minor version of o.a.j.oak.commons is so problematic. It should only affect bundles that implement classes/interfaces from this package. Consumers of the package should be unaffected (default import version range is [2,3)).

The version for this package has been repeatedly bumped in the last 2 years.

@reschke do you have any specific concerns? Or just a general dislike for bumping versions?

I am certainly with you when it comes to being prudent with version bumps. But in order to move forward, we sometimes need to do them. WDYT?

[reschke]

I'm unhappy changing public APIs with no need.

Note: "public APIs" == packages that are exported and not marked "Internal".

[reschke]

if something which is not supposed to be used outside Oak isn't marked "internal", we should fix that.

[lweitzendorf]

Now that the last affected package is marked as internal, I've updated the PR to replace all uses of Buffer with ByteBuffer and not touch oak-commons other than the deprecation itself.

[reschke]

1.22.x still needs to support Java 8.

That's why we need to be careful with changing APIs.

[reschke]

If there is a remote chance to split this into smaller pieces, please do so. For instance, as a first step, only make those changes, that can be done independently of the API changes. If possible.