Skip to content

IGNITE-28848 ZooKeeper discovery: stream marshalZip through DeflaterOutputStream to cut allocations and peak memory#13319

Merged
anton-vinogradov merged 4 commits into
apache:masterfrom
anton-vinogradov:ignite-zk-marshalzip-stream
Jul 14, 2026
Merged

IGNITE-28848 ZooKeeper discovery: stream marshalZip through DeflaterOutputStream to cut allocations and peak memory#13319
anton-vinogradov merged 4 commits into
apache:masterfrom
anton-vinogradov:ignite-zk-marshalzip-stream

Conversation

@anton-vinogradov

@anton-vinogradov anton-vinogradov commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

JIRA: IGNITE-28848 (follow-up to IGNITE-28835)

Why this matters. marshalZip() sits on the two busiest control-plane paths of ZooKeeper discovery: the coordinator re-marshals the whole ZkDiscoveryEventsData on every discovery event (node joins/failures and every custom message — cache create/destroy, exchange, services, snapshots, ...), and join data / data-for-joined is megabytes in clusters with hundreds of caches (split across znodes by jute.maxbuffer). The current code materializes the uncompressed form before compressing: ~6x the uncompressed size in garbage and ~3x at peak per call — so a mass join, client reconnect storm or rolling restart hits the coordinator with GC pressure and heap spikes exactly when it is busiest, and the pauses land on the discovery critical path (events are processed sequentially, the whole cluster sees slower event propagation). Deflater.end() is never called either, so zlib native memory waits for the Cleaner.

Change: stream marshalling straight through BufferedOutputStream -> DeflaterOutputStream: the uncompressed form is never materialized, peak memory is bounded by the compressed output, and the deflater is released deterministically on close. The BufferedOutputStream is essential — ObjectOutputStream writes ~1 KB blocks, and unbuffered per-block JNI deflate calls cost +14–20% time. The private zip() helper is removed (marshalZip was its only caller); unzip() stays (still used for ATTR_SECURITY_SUBJECT_V2); the read side (unmarshalZip) already streams. Same pattern as DiscoveryMessageParser.marshalZip in the same package.

Compatibility: zlib format is unchanged (default Deflater settings in both versions) — old nodes inflate the bytes as is, rolling upgrade safe.

JMH (avgt, JDK 17, -prof gc), old vs new:

payload (marshalled → zipped) time old, us/op time new, us/op alloc old, B/op alloc new, B/op
MAP_2K (1.6 KB → 0.7 KB) 21.3 ± 11.5 15.4 ± 0.7 12,544 14,368 (+15%)
MAP_100K (132 KB → 40 KB) 1,987 ± 512 1,931 ± 264 1,065,204 283,835 (−73%)
LIST_1M (1.66 MB → 318 KB) 17,661 ± 3,433 16,825 ± 980 9,891,984 1,788,235 (−82%)

Allocations drop 73–82% on realistic payloads; per-call time is on par or better. The small-payload trade-off (+15% B/op — bounded fixed overhead of the 8 KB buffer, break-even ~3 KB) is analyzed in the JIRA description.

🤖 Generated with Claude Code

…utputStream to cut allocations and peak memory

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
anton-vinogradov and others added 3 commits July 14, 2026 17:09
Reword the comment so it is clear the 8 KB BufferedOutputStream buffer
coalesces JdkMarshaller's ~1 KB ObjectOutputStream block-data writes,
rather than reading as a 1 KB buffer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iew)

Pre-size the compressed-output GridByteArrayOutputStream to 8 KB (the
BufferedOutputStream buffer size) so the stream no longer grows through a
chain of doubling reallocations. The uncompressed size is intentionally not
computed anymore, so the seed is a fixed capacity rather than a ratio of the
input length.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…urn (review)"

This reverts commit 7d40304. The uncompressed size isn't available in
the streaming path, so the seed could only be a fixed constant rather than
proportional to the input; per review, keep the default GridByteArrayOutputStream
sizing instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@anton-vinogradov anton-vinogradov merged commit f18ea71 into apache:master Jul 14, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants