iamaleksey commented on code in PR #1831:
URL: https://github.com/apache/cassandra/pull/1831#discussion_r963861095
##########
src/java/org/apache/cassandra/schema/SchemaKeyspace.java:
##########
@@ -961,31 +960,33 @@ private static TableMetadata fetchTable(String
keyspaceName, String tableName, T
@VisibleForTesting
static TableParams createTableParamsFromRow(UntypedResultSet.Row row)
{
- return TableParams.builder()
- // allow_auto_snapshot column was introduced in 4.2
- .allowAutoSnapshot(row.has("allow_auto_snapshot") ?
-
row.getBoolean("allow_auto_snapshot", DatabaseDescriptor.isAutoSnapshot()) :
-
DatabaseDescriptor.isAutoSnapshot())
-
.bloomFilterFpChance(row.getDouble("bloom_filter_fp_chance"))
-
.caching(CachingParams.fromMap(row.getFrozenTextMap("caching")))
- .comment(row.getString("comment"))
-
.compaction(CompactionParams.fromMap(row.getFrozenTextMap("compaction")))
-
.compression(CompressionParams.fromMap(row.getFrozenTextMap("compression")))
- .memtable(MemtableParams.get(row.has("memtable") ?
row.getString("memtable") : null)) // memtable column was introduced in 4.1
-
.defaultTimeToLive(row.getInt("default_time_to_live"))
- .extensions(row.getFrozenMap("extensions",
UTF8Type.instance, BytesType.instance))
- .gcGraceSeconds(row.getInt("gc_grace_seconds"))
- .maxIndexInterval(row.getInt("max_index_interval"))
-
.memtableFlushPeriodInMs(row.getInt("memtable_flush_period_in_ms"))
- .minIndexInterval(row.getInt("min_index_interval"))
- .crcCheckChance(row.getDouble("crc_check_chance"))
-
.speculativeRetry(SpeculativeRetryPolicy.fromString(row.getString("speculative_retry")))
-
.additionalWritePolicy(row.has("additional_write_policy") ?
-
SpeculativeRetryPolicy.fromString(row.getString("additional_write_policy")) :
-
SpeculativeRetryPolicy.fromString("99PERCENTILE"))
- .cdc(row.has("cdc") && row.getBoolean("cdc"))
- .readRepair(getReadRepairStrategy(row))
- .build();
+ TableParams.Builder builder = TableParams.builder()
+
.bloomFilterFpChance(row.getDouble("bloom_filter_fp_chance"))
+
.caching(CachingParams.fromMap(row.getFrozenTextMap("caching")))
+
.comment(row.getString("comment"))
+
.compaction(CompactionParams.fromMap(row.getFrozenTextMap("compaction")))
+
.compression(CompressionParams.fromMap(row.getFrozenTextMap("compression")))
+
.memtable(MemtableParams.get(row.has("memtable") ? row.getString("memtable") :
null)) // memtable column was introduced in 4.1
+
.defaultTimeToLive(row.getInt("default_time_to_live"))
+
.extensions(row.getFrozenMap("extensions", UTF8Type.instance,
BytesType.instance))
+
.gcGraceSeconds(row.getInt("gc_grace_seconds"))
+
.maxIndexInterval(row.getInt("max_index_interval"))
+
.memtableFlushPeriodInMs(row.getInt("memtable_flush_period_in_ms"))
+
.minIndexInterval(row.getInt("min_index_interval"))
+
.crcCheckChance(row.getDouble("crc_check_chance"))
+
.speculativeRetry(SpeculativeRetryPolicy.fromString(row.getString("speculative_retry")))
+
.additionalWritePolicy(row.has("additional_write_policy") ?
+
SpeculativeRetryPolicy.fromString(row.getString("additional_write_policy")) :
+
SpeculativeRetryPolicy.fromString("99PERCENTILE"))
+ .cdc(row.has("cdc") &&
row.getBoolean("cdc"))
+
.readRepair(getReadRepairStrategy(row));
+
+ // allow_auto_snapshot column was introduced in 4.2
+ if (row.has("allow_auto_snapshot")) {
+ builder.allowAutoSnapshot(row.getBoolean("allow_auto_snapshot"));
+ }
Review Comment:
Either put braces on a new line or don't do braces at all? According to our
style guide. I would prefer the latter, no braces.
##########
src/java/org/apache/cassandra/schema/SchemaKeyspace.java:
##########
@@ -568,8 +568,7 @@ private static void addTableParamsToRowBuilder(TableParams
params, Row.SimpleBui
// As above, only add the allow_auto_snapshot column if the value is
not default (true) and
// auto-snapshotting is enabled, to avoid RTE in pre-4.2 versioned
node during upgrades
- if (DatabaseDescriptor.isAutoSnapshot() && !params.allowAutoSnapshot)
- builder.add("allow_auto_snapshot", false);
+ builder.add("allow_auto_snapshot", params.allowAutoSnapshot);
Review Comment:
You still want to gate it on `!params.allowAutoSnapshot`, so it's only
persisted if explicitly set to `false`. Just don't involve
`DatabaseDescriptor.isAutoSnapshot()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
