Skip to content

Commit 647ed4f

Browse files
CASSANDRA-21134: Make direct-write classification total over OperationType
RELOCATE (online relocateSSTables) and UNKNOWN (offline sstablesplit) reach DataComponent.buildWriter() despite writesData==false, so keying the direct-write classification off writesData left RELOCATE unclassified and threw under background_write_disk_access_mode: direct. Make DIRECT_WRITE_SUPPORT total over OperationType.values(): classify RELOCATE and UNKNOWN as SUPPORTED (both append-only, safe under O_DIRECT) and the nine genuine non-writers as a new NOT_A_WRITER marker. The build-time guard now asserts completeness over values(), so a newly-added OperationType fails at class-load instead of in production.
1 parent 104a65d commit 647ed4f

3 files changed

Lines changed: 43 additions & 16 deletions

File tree

src/java/org/apache/cassandra/io/DirectIoSupport.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,13 @@ public enum DirectIoSupport
4040
* Direct IO would work but is deliberately disabled for performance or cache-residency
4141
* reasons. Removing this exclusion requires re-evaluating the policy, not code changes.
4242
*/
43-
UNSUPPORTED_POLICY;
43+
UNSUPPORTED_POLICY,
44+
45+
/**
46+
* The operation never constructs a data-file writer, so the direct-IO question does not apply.
47+
* Exists so a consumer's classification can be total over its operation enum rather than partial.
48+
*/
49+
NOT_A_WRITER;
4450

4551
public boolean isSupported()
4652
{

src/java/org/apache/cassandra/io/sstable/format/DataComponent.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.apache.cassandra.schema.CompressionParams;
4141
import org.apache.cassandra.schema.TableMetadata;
4242

43+
import static org.apache.cassandra.io.DirectIoSupport.NOT_A_WRITER;
4344
import static org.apache.cassandra.io.DirectIoSupport.SUPPORTED;
4445
import static org.apache.cassandra.io.DirectIoSupport.UNSUPPORTED_CORRECTNESS;
4546
import static org.apache.cassandra.io.DirectIoSupport.UNSUPPORTED_POLICY;
@@ -62,16 +63,35 @@ private static EnumMap<OperationType, DirectIoSupport> buildDirectWriteSupport()
6263
m.put(OperationType.COMPACTION, SUPPORTED);
6364
m.put(OperationType.TOMBSTONE_COMPACTION, SUPPORTED);
6465
m.put(OperationType.STREAM, SUPPORTED);
66+
// writesData==false yet these still reach buildWriter() (RELOCATE via relocateSSTables, UNKNOWN via
67+
// the offline sstablesplit tool), which is why classification can't key off writesData. CASSANDRA-21134.
68+
m.put(OperationType.RELOCATE, SUPPORTED);
69+
m.put(OperationType.UNKNOWN, SUPPORTED);
6570

6671
// tryAppend() needs mark()/resetAndTruncate(), unsupported under O_DIRECT.
6772
m.put(OperationType.SCRUB, UNSUPPORTED_CORRECTNESS);
6873

6974
// Flushed data is hot; keep it in the page cache.
7075
m.put(OperationType.FLUSH, UNSUPPORTED_POLICY);
7176

77+
// These never construct a data-file writer (read-only, rewrite a non-data component, or write their
78+
// own files), so they never reach buildWriter(). Classified only to keep the map total.
79+
m.put(OperationType.P0, NOT_A_WRITER);
80+
m.put(OperationType.VERIFY, NOT_A_WRITER);
81+
m.put(OperationType.VALIDATION, NOT_A_WRITER);
82+
m.put(OperationType.INDEX_BUILD, NOT_A_WRITER);
83+
m.put(OperationType.VIEW_BUILD, NOT_A_WRITER);
84+
m.put(OperationType.INDEX_SUMMARY, NOT_A_WRITER);
85+
m.put(OperationType.KEY_CACHE_SAVE, NOT_A_WRITER);
86+
m.put(OperationType.ROW_CACHE_SAVE, NOT_A_WRITER);
87+
m.put(OperationType.COUNTER_CACHE_SAVE, NOT_A_WRITER);
88+
89+
// Total over the enum so a newly-added OperationType fails here at class-load instead of throwing
90+
// from buildWriter() in production (CASSANDRA-21134: RELOCATE slipped through the old writesData gate).
7291
for (OperationType op : OperationType.values())
73-
if (op.writesData && !m.containsKey(op))
74-
throw new AssertionError("Missing direct-write classification for " + op);
92+
if (!m.containsKey(op))
93+
throw new AssertionError("Missing direct-write classification for " + op
94+
+ " — every OperationType must be SUPPORTED, UNSUPPORTED_*, or NOT_A_WRITER");
7595
return m;
7696
}
7797

@@ -164,8 +184,7 @@ static boolean isDirectWriteSupported(OperationType operationType)
164184
{
165185
DirectIoSupport support = DIRECT_WRITE_SUPPORT.get(operationType);
166186
if (support == null)
167-
throw new IllegalArgumentException("OperationType " + operationType
168-
+ " has no direct-write classification — likely a read-only operation routed through buildWriter()");
187+
throw new IllegalStateException("OperationType " + operationType + " has no direct-write classification");
169188
return support.isSupported();
170189
}
171190

test/unit/org/apache/cassandra/io/sstable/format/DataComponentDirectWriteSelectionTest.java

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,11 @@ public void tearDown()
7979
}
8080

8181
@Test
82-
public void testWriterSelectionMatchesAllowListForEveryDataWritingOp() throws Exception
82+
public void testWriterSelectionMatchesAllowListForEveryOp() throws Exception
8383
{
8484
DirectIoTestUtils.withDirectWrites(() -> {
8585
for (OperationType op : OperationType.values())
8686
{
87-
if (!op.writesData)
88-
continue;
89-
9087
boolean expectDirect = DataComponent.isDirectWriteSupported(op);
9188
try (SequentialWriter w = build(compressedMetadata(), op))
9289
{
@@ -99,7 +96,7 @@ public void testWriterSelectionMatchesAllowListForEveryDataWritingOp() throws Ex
9996
}
10097

10198
// The allow-list loop test takes its oracle from isDirectWriteSupported, so it can't catch a bad
102-
// reclassification of these two ops. Pin their intent independently.
99+
// reclassification of these ops. Pin their intent independently.
103100
@Test
104101
public void testScrubAndFlushAreNeverDirectWriteSupported()
105102
{
@@ -109,15 +106,23 @@ public void testScrubAndFlushAreNeverDirectWriteSupported()
109106
DataComponent.isDirectWriteSupported(OperationType.FLUSH));
110107
}
111108

109+
// RELOCATE and UNKNOWN are writesData==false but DO reach buildWriter (relocateSSTables / offline
110+
// sstablesplit). Pin them SUPPORTED so they can't silently regress to NOT_A_WRITER (CASSANDRA-21134).
111+
@Test
112+
public void testRelocateAndUnknownAreDirectWriteSupported()
113+
{
114+
assertTrue("RELOCATE rewrites SSTables via a compaction task and must use direct writes",
115+
DataComponent.isDirectWriteSupported(OperationType.RELOCATE));
116+
assertTrue("UNKNOWN reaches buildWriter via the offline sstablesplit tool and must use direct writes",
117+
DataComponent.isDirectWriteSupported(OperationType.UNKNOWN));
118+
}
119+
112120
@Test
113121
public void testUncompressedAlwaysUsesStandardWriter() throws Exception
114122
{
115123
DirectIoTestUtils.withDirectWrites(() -> {
116124
for (OperationType op : OperationType.values())
117125
{
118-
if (!op.writesData)
119-
continue;
120-
121126
try (SequentialWriter w = build(uncompressedMetadata(), op))
122127
{
123128
assertTrue("Uncompressed tables must use ChecksummedSequentialWriter for " + op + ", got " + w.getClass().getSimpleName(),
@@ -136,9 +141,6 @@ public void testBufferedModeNeverPicksDirectWriter()
136141
{
137142
for (OperationType op : OperationType.values())
138143
{
139-
if (!op.writesData)
140-
continue;
141-
142144
try (SequentialWriter w = build(compressedMetadata(), op))
143145
{
144146
assertTrue("Buffered mode must not produce DirectCompressedSequentialWriter for " + op + ", got " + w.getClass().getSimpleName(),

0 commit comments

Comments
 (0)