Mmuzaf commented on code in PR #2046:
URL: https://github.com/apache/cassandra/pull/2046#discussion_r1167095097
##########
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java:
##########
@@ -352,14 +410,166 @@
* This is an optimization used in unit tests becuase we never restart a
node there. The only node is stopoped
* when the JVM terminates. Therefore, we can use such optimization and
not wait unnecessarily. */
NON_GRACEFUL_SHUTDOWN("cassandra.test.messagingService.nonGracefulShutdown"),
+ CHRONICLE_ANNOUNCER_DISABLE("chronicle.announcer.disable"),
+
COMMITLOG_ALLOW_IGNORE_SYNC_CRC("cassandra.commitlog.allow_ignore_sync_crc"),
+ COMMITLOG_IGNORE_REPLAY_ERRORS("cassandra.commitlog.ignorereplayerrors"),
+
COMMITLOG_MAX_OUTSTANDING_REPLAY_BYTES("cassandra.commitlog_max_outstanding_replay_bytes",
String.valueOf(1024 * 1024 * 64)),
+
COMMITLOG_MAX_OUTSTANDING_REPLAY_COUNT("cassandra.commitlog_max_outstanding_replay_count",
"1024"),
+ COMMITLOG_STOP_ON_ERRORS("cassandra.commitlog.stop_on_errors"),
+ CONFIG_LOADER("cassandra.config.loader"),
+ CONSISTENT_RANGE_MOVEMENT("cassandra.consistent.rangemovement", "true"),
+
CONSISTENT_SIMULTANEOUS_MOVES_ALLOW("cassandra.consistent.simultaneousmoves.allow",
"false"),
+
CUSTOM_GUARDRAILS_CONFIG_PROVIDER_CLASS("cassandra.custom_guardrails_config_provider_class"),
+ CUSTOM_QUERY_HANDLER_CLASS("cassandra.custom_query_handler_class"),
+ CUSTOM_TRACING_CLASS("cassandra.custom_tracing_class"),
+
DATA_OUTPUT_STREAM_PLUS_TEMP_BUFFER_SIZE("cassandra.data_output_stream_plus_temp_buffer_size",
"8192"),
+
DECAYING_ESTIMATED_HISTOGRAM_RESERVOIR_STRIPE_COUNT("cassandra.dehr_stripe_count",
"2"),
+
DIAGNOSTIC_SNAPSHOT_INTERVAL_NANOS("cassandra.diagnostic_snapshot_interval_nanos",
"60000000000"),
+
DISABLE_AUTH_CACHES_REMOTE_CONFIGURATION("cassandra.disable_auth_caches_remote_configuration"),
+ DISABLE_PAXOS_AUTO_REPAIRS("cassandra.disable_paxos_auto_repairs"),
+ DISABLE_PAXOS_STATE_FLUSH("cassandra.disable_paxos_state_flush"),
+ DISABLE_STCS_IN_L0("cassandra.disable_stcs_in_l0"),
+ DISABLE_TCACTIVE_OPENSSL("cassandra.disable_tcactive_openssl"),
+ DOB_DOUBLING_THRESHOLD_MB("cassandra.DOB_DOUBLING_THRESHOLD_MB", "64"),
+ DOB_MAX_RECYCLE_BYTES("cassandra.dob_max_recycle_bytes",
String.valueOf(1024 * 1024)),
+
DROP_OVERSIZED_READ_REPAIR_MUTATIONS("cassandra.drop_oversized_readrepair_mutations"),
+ DTEST_API_LOG_TOPOLOGY("cassandra.dtest.api.log.topology"),
+ ENABLE_DC_LOCAL_COMMIT("cassandra.enable_dc_local_commit", "true"),
+
EXPIRATION_DATE_OVERFLOW_POLICY("cassandra.expiration_date_overflow_policy"),
+
EXPIRATION_OVERFLOW_WARNING_INTERVAL_MINUTES("cassandra.expiration_overflow_warning_interval_minutes",
"5"),
+ FD_INITIAL_VALUE_MS("cassandra.fd_initial_value_ms"),
+ FD_MAX_INTERVAL_MS("cassandra.fd_max_interval_ms"),
+ FILE_CACHE_ENABLED("cassandra.file_cache_enabled"),
+
FORCE_DEFAULT_INDEXING_PAGE_SIZE("cassandra.force_default_indexing_page_size"),
+ FORCE_PAXOS_STATE_REBUILD("cassandra.force_paxos_state_rebuild"),
+ GIT_SHA("cassandra.gitSHA"),
+
GOSSIP_DISABLE_THREAD_VALIDATION("cassandra.gossip.disable_thread_validation",
"false"),
+ IGNORE_CORRUPTED_SCHEMA_TABLES("cassandra.ignore_corrupted_schema_tables",
"false"),
Review Comment:
@smiklosovic , @jacek-lewandowski
Folks, I'd like to continue with this as I've found some inconsistencies in
the source code regarding the use of defaults. I'd like to discuss with you
some thoughts about the `CassandraRelevantProperties` default values. My
concern here is that the current API for getting the value of system properties
doesn't give us a reliable and explicit way to get the same results each time
we access a property, so removing "redundant" defaults may cause more problems
than adding them explicitly.
### Current state
Let's take a look at a few examples and inconsistencies:
1. Inconsistency between `getBoolean()` and `getInt()`/`getLong()` methods:
* in case `getBoolean()` without defaults value the `false` value returned,
no exception is thrown;
* in case `getInt()` or `getLong()` with default value the exception is
thrown (`NullPointerException` :-( see
[testInteger_null()](https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/config/CassandraRelevantPropertiesTest.java#L133)
test);
2. Inconsistency between `getBoolean()` and `getString()` for the same
property (assume a system property with type boolean is used):
* `getBoolean()` will return a non-null value if the default value is `null`;
* `getString()` for the same property will return a `null` value;
3. If the default value is `null` the `NullPointerException` is thrown:
The `getBoolean()` method implicitly returns the `false` value and depends
on the boolean converter implementation itself (so a developer has to delve
into the code to find the _real_ return value). However, the `public long
getLong()` will throw `NullPointerException`
### Expected state
So, I think the API should be the following (default value `null` is
allowed) and we have to fix it:
1. `boolean disableSTCSInL0 = DISABLE_STCS_IN_L0.getBoolean(false);`
(runtime option)
- If the property is set, the corresponding system property value is used;
- If the property is NOT set, the default value from brackets is used;
_OR_
2. `DISABLE_STCS_IN_L0("cassandra.disable_stcs_in_l0", "false")` (compile
time option)
- If the property is set, the corresponding system property value is used;
- If the property is NOT set, the default value from the constant is used;
_OR_
3. `boolean disableSTCSInL0 = DISABLE_STCS_IN_L0.getBoolean()` (property is
expected to be set)
- If the property is set, the corresponding system property value is used;
- If the property is NOT set, and there is NO default value we MUST throw an
exception.
WDYT?
--
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]
