[ https://issues.apache.org/jira/browse/CASSANDRA-15234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17152159#comment-17152159 ]
Ekaterina Dimitrova edited comment on CASSANDRA-15234 at 7/6/20, 5:06 PM: -------------------------------------------------------------------------- Apologize for my late response. I was a bit sick these days and tried to disengage from work and take some rest over the weekend. With all my respect to everyone's opinion and experience on the project, I have two points here: - I truly support [~mck]'s questions. I believe they should be responded before any decision is taken and someone jumps into actual work. {quote}how many settings does it apply to? is taxonomy based on a technical or user perspective? if user/operator based, how many people need to be involved to get it right? if user/operator based, what if one property applies to multiple concerns? how does the @Replace annotation work between levels in the grouping? does this introduce more complexity/variations in what has to be tested? (since yaml can consist of old and new setting names) {quote} - I was also wondering today while I was trying to be open-minded and look from all perspectives at this ticket/patch... Did anyone check the first [commit |https://github.com/ekaterinadimitrova2/cassandra/blob/CASSANDRA-15234-1-outdated/conf/cassandra.yaml] where I was suggesting reorganizing of the text into the yaml into sections? I also put it into the ticket thread . This was a quick draft shared two months ago that could be reworked to sections that satisfy the users' requirements for clarity and consistency. Do we see any big difference for the users between: {code:java} #*Replica Filtering Protection* cached_rows_warn_threshold: 1000 cached_rows_fail_threshold: 16000 {code} and: {code:java} replica_filtering_protection: - cached_rows_warn_threshold: 1000 - cached_rows_fail_threshold: 16000 {code} >From that perspective, I think the C* community can accept this patch and then >we can raise a new ticket) to improve the internals from our engineering >perspective in Beta(refactoring the Config class and the backward >compatibility framework), as suggested by [~mck]. I think this work could be >really considered incremental work. Having that in mind, honestly, I don't find a justification to spend my time to rework and fully re-test the patch at this point in time. I am fine to be proved wrong in a justified way. [~benedict], [~blerer], [~mck], do you agree with me on my suggestion(reorganizing the yaml file and doing the nested parameters approach later)? {quote}I think this is indeed preferable to releasing an API we already expect to deprecate, however I think we're overstating the difficulty here. We haven't debated the parameter naming much at all, and we can easily land this in 4.0-beta. If [~e.dimitrova] doesn't have the time, and 4.0-beta is an acceptable window to land the work, I can take a look in a few weeks. \{quote} I want to be clear - it is not about difficulty, this patch is time consuming. It needs attention to the detail and look at the whole config which touches the code at many places(also ccm, dtests, in-jvm tests, etc) was (Author: e.dimitrova): Apologize for my late response. I was a bit sick these days and tried to disengage from work and take some rest over the weekend. With all my respect to everyone's opinion and experience on the project, I have two points here: - I truly support [~mck]'s questions. I believe they should be responded before any decision is taken and someone jumps into actual work. {quote}how many settings does it apply to? is taxonomy based on a technical or user perspective? if user/operator based, how many people need to be involved to get it right? if user/operator based, what if one property applies to multiple concerns? how does the @Replace annotation work between levels in the grouping? does this introduce more complexity/variations in what has to be tested? (since yaml can consist of old and new setting names) {quote} - I was also wondering today while I was trying to be open-minded and look from all perspectives at this ticket/patch... Did anyone check the first [commit |https://github.com/ekaterinadimitrova2/cassandra/blob/CASSANDRA-15234-1-outdated/conf/cassandra.yaml] where I was suggesting reorganizing of the text into the yaml into sections? I also put it into the ticket thread . This was a quick draft shared two months ago that could be reworked to sections that satisfy the users' requirements for clarity and consistency. Do we see any big difference for the users between: {code:java} #*Replica Filtering Protection* cached_rows_warn_threshold: 1000 cached_rows_fail_threshold: 16000 {code} and: {code:java} replica_filtering_protection: - cached_rows_warn_threshold: 1000 - cached_rows_fail_threshold: 16000 {code} >From that perspective, I think the C* community can accept this patch and then >we can raise a new ticket) to improve the internals from our engineering >perspective in Beta(refactoring the Config class and the backward >compatibility framework), as suggested by [~mck]. I think this work could be >really considered incremental work. Having that in mind, honestly, I don't find a justification to spend my time to rework and fully re-test the patch at this point in time. I am fine to be proved wrong in a justified way. [~benedict], [~blerer], [~mck], do you agree with me on my suggestion(reorganizing the yaml file and doing the nested parameters approach later)? {quote} I think this is indeed preferable to releasing an API we already expect to deprecate, however I think we're overstating the difficulty here. We haven't debated the parameter naming much at all, and we can easily land this in 4.0-beta. If [~e.dimitrova] doesn't have the time, and 4.0-beta is an acceptable window to land the work, I can take a look in a few weeks. \{quote} I want to be clear - it is not about difficulty, this patch is time consuming. It needs attention to the detail and look at the whole config which touches the code at many places(also ccm, dtests, in-jvm tests, etc) > Standardise config and JVM parameters > ------------------------------------- > > Key: CASSANDRA-15234 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15234 > Project: Cassandra > Issue Type: Bug > Components: Local/Config > Reporter: Benedict Elliott Smith > Assignee: Ekaterina Dimitrova > Priority: Normal > Fix For: 4.0-alpha > > Attachments: CASSANDRA-15234-3-DTests-JAVA8.txt > > > We have a bunch of inconsistent names and config patterns in the codebase, > both from the yams and JVM properties. It would be nice to standardise the > naming (such as otc_ vs internode_) as well as the provision of values with > units - while maintaining perpetual backwards compatibility with the old > parameter names, of course. > For temporal units, I would propose parsing strings with suffixes of: > {{code}} > u|micros(econds?)? > ms|millis(econds?)? > s(econds?)? > m(inutes?)? > h(ours?)? > d(ays?)? > mo(nths?)? > {{code}} > For rate units, I would propose parsing any of the standard {{B/s, KiB/s, > MiB/s, GiB/s, TiB/s}}. > Perhaps for avoiding ambiguity we could not accept bauds {{bs, Mbps}} or > powers of 1000 such as {{KB/s}}, given these are regularly used for either > their old or new definition e.g. {{KiB/s}}, or we could support them and > simply log the value in bytes/s. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org