[
https://issues.apache.org/jira/browse/CASSANDRA-21462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18103185#comment-18103185
]
Jon Haddad edited comment on CASSANDRA-21462 at 8/9/26 8:38 PM:
----------------------------------------------------------------
Addressing review comments:
- e5dbc34e8d — Swapped com.sun.management.ThreadMXBean for the existing
ThreadStats wrapper in the allocation gate test, adding
isThreadAllocatedMemorySupported() so it skips cleanly on JVMs without
thread-allocation tracking instead of silently reading 0 bytes.
- a0b434c044 — Replaced Thread.sleep-past-TTL-boundary patterns with
deterministic controls: CompactionTask gained an injectable
nowInSecondsSupplier, exposed through
DifferentialCompactionTester.taskWithFixedNow and LongSupplier-taking
overloads; purge-boundary tests now read the actual local deletion time and
pass an explicit gcBefore instead of sleeping.
- 8bd22bbf37 — New differential coverage for previously-untested primary-key
shapes: large (100s-1000s of bytes) single-column and composite partition keys,
a clustering column at the 128-byte vint boundary, and frozen UDTs/collections
used as the partition or clustering key (not just as a regular column).
- 478c6cfd28 — Named a magic value (0b10 → CLUSTERING_VALUE_FLAG_NULL) in
ClusteringComparator's wire comparator.
A few additional things I found, Hot-path allocation/CPU trims (fleet review,
separate from the PR feedback):
- 4ea08c1994 — Three fixes: BigCursorIndexWriter avoided boxing via getInt(i)
instead of .get(i); DeletionTime.equals/supersedes/isLive compare raw fields
directly instead of re-deriving longs through virtual calls each time;
CursorCompactor's merge-order sort got a StatefulCursor-specialized dispatch to
keep the hot compare call site monomorphic instead of megamorphic across 4
comparator implementations. Measured allocation-gate deltas included.
- c22ec79580 — Precomputed a long[] droppedTimeArray per column superset
instead of a per-cell ByteBuffer-keyed map lookup for dropped-column checks.
Caught an inverted sentinel bug immediately via
DroppedColumnDifferentialCompactionTest before landing. New JMH bench
(CompactionDroppedColumnBench) shows the drop-column-specific overhead dropping
from ~6.8ms to ~1.2ms.
- e896f1577d — Replaced a per-cell cellColumn.isComplex() branch (dead code —
cellPath is never read downstream, and complex columns are already gated out)
with an assertion.
- 11ddd55e1d — Removed redundant dataWriter.position() re-reads in writeRowEnd
(up to 3 per row), reusing already-captured values; the position-verifying
check moved into the assert expression so it's free with assertions disabled.
was (Author: rustyrazorblade):
Addressing review comments:
- e5dbc34e8d — Swapped com.sun.management.ThreadMXBean for the existing
ThreadStats wrapper in the allocation gate test, adding
isThreadAllocatedMemorySupported() so it skips cleanly on JVMs without
thread-allocation tracking instead of silently reading 0 bytes.
- a0b434c044 — Replaced Thread.sleep-past-TTL-boundary patterns with
deterministic controls: CompactionTask gained an injectable
nowInSecondsSupplier, exposed through
DifferentialCompactionTester.taskWithFixedNow and LongSupplier-taking
overloads; purge-boundary tests now read the actual local deletion time and
pass an explicit gcBefore instead of sleeping.
- 8bd22bbf37 — New differential coverage for previously-untested primary-key
shapes: large (100s-1000s of bytes) single-column and composite partition keys,
a clustering column at the 128-byte vint boundary, and frozen UDTs/collections
used as the partition or clustering key (not just as a regular column).
- 478c6cfd28 — Named a magic value (0b10 → CLUSTERING_VALUE_FLAG_NULL) in
ClusteringComparator's wire comparator.
A few additional things I found, Hot-path allocation/CPU trims (fleet review,
separate from the PR feedback):
- 4ea08c1994 — Three fixes: BigCursorIndexWriter avoided boxing via getInt(i)
instead of .get(i); DeletionTime.equals/supersedes/isLive compare raw fields
directly instead of re-deriving longs through virtual calls each time;
CursorCompactor's merge-order sort got a StatefulCursor-specialized dispatch to
keep the hot compare call site monomorphic instead of megamorphic across 4
comparator implementations. Measured allocation-gate deltas included.
- c22ec79580 — Precomputed a long[] droppedTimeArray per column superset
instead of a per-cell ByteBuffer-keyed map lookup for dropped-column checks.
Caught an inverted sentinel bug immediately via
DroppedColumnDifferentialCompactionTest before landing. New JMH bench
(CompactionDroppedColumnBench) shows the drop-column-specific overhead dropping
from ~6.8ms to ~1.2ms.
- e896f1577d — Replaced a per-cell cellColumn.isComplex() branch (dead code —
cellPath is never read downstream, and complex columns are already gated out)
with an assertion.
- 11ddd55e1d — Removed redundant dataWriter.position() re-reads in writeRowEnd
(up to 3 per row), reusing already-captured values; the position-verifying
check moved into the assert expression so it's free with assertions disabled.
> Add byte for byte test harness to detect and resolve cursor deviations from
> iterator path
> -----------------------------------------------------------------------------------------
>
> Key: CASSANDRA-21462
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21462
> Project: Apache Cassandra
> Issue Type: Sub-task
> Components: Local/Compaction
> Reporter: Jon Haddad
> Assignee: Jon Haddad
> Priority: High
> Fix For: 6.0
>
>
> This issue addresses various deviations and inconsistencies with the cursor
> path by introducing tests that verify both the old iterator path and the new
> cursor path generate the same outputs given the same inputs. It also fixes
> various issues found in the process of creating this test system that
> resulted in corruption or incorrect values being carried into the resulting
> SSTable.
> It adds a test that uses the JVM's internal memory tracking to assert that we
> do NOT do unnecessary allocations in the cursor path that should guard
> against future regressions, and fixes an unnecessary enum allocation.
> It does NOT add support for BTI, multi-cell columns, counters, etc.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
