Mmuzaf commented on code in PR #2046:
URL: https://github.com/apache/cassandra/pull/2046#discussion_r1177762095


##########
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:
   @jacek-lewandowski @smiklosovic 
   
   I have prototyped a configuration approach you suggested and it looks good 
at first glance. So, we could have multiple configuration sources under the 
same interface:
   - `Config` class;
   - `CassandraRelevantProperties`
   - `CassandraRelevantEnv`
   
   Having at the same time a read-only access layer represented with 
`ConfigurationRegistry` + `ConfgurtionValue<T>`, we will be able to turn the 
following source code:
   
   ```
   if (conf.auto_snapshot_ttl != null)
   {
       try
       {
           holder.autoSnapshoTtl = new 
DurationSpec.IntSecondsBound(conf.auto_snapshot_ttl);
       }
       catch (IllegalArgumentException e)
       {
           throw new ConfigurationException("Invalid value of 
auto_snapshot_ttl: " + conf.auto_snapshot_ttl, false);
       }
   }
   ```
   
   into
   
   ```
   registry.getValue(String.class, "auto_snapshot_ttl")
           .map(DurationSpec.IntSecondsBound::new,
                (e, v) -> new ConfigurationException("Invalid value of 
auto_snapshot_ttl: " + v, false))
           .ifPresent(v -> holder.autoSnapshoTtl = v);
   ```
   
   It looks much better and suits our needs. Here is the prototype of the 
`ConfigurationValue<T>` class:
   
https://github.com/apache/cassandra/compare/trunk...Mmuzaf:cassandra:cassandra-15254-apply#diff-16d8aa644f29a8e76deedbb8dd760530530c90d4dd14ad4c4ea60823b84ea79dR31
   
   
   I guess we could implement that in another issue (cassandra-15254?), so here 
we just need to decide what to do with the CRP default values.
   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]

Reply via email to