Hello everyone, I have created the following JIRA issues to follow up on the discussion and to improve the user experience with virtual tables. I'll try to address them before the next release.
CASSANDRA-18586: CQLSH formatting output is incorrect when querying the system properties virtual table https://issues.apache.org/jira/browse/CASSANDRA-18586 CASSANDRA-18587: Align all system property names used by the project to use the 'cassandra' prefix https://issues.apache.org/jira/browse/CASSANDRA-18587 I want also to clarify and discuss with you some points related to the SystemPropertiesTable virtual table. 1. Hide non-production environment properties in the SystemProperties virtual table. As you may know, the result of the query on the virtual table currently includes the environment properties related to tests as well (used internally for our testing purposes) ~ 42 out of 290. This seems a bit redundant for production use and floods the query output with the things you don't need to think about. I think we can add a new property -Dcassandra.vt.show.test.system.properties (false by default) properties to hide these test-related properties for production environments while still using them for our test runs and build scripts. Hiding test properties is not a regression in this case. Any thoughts? 2. The absence of a description of the system properties. I have found that there is no good description of the system properties used to configure production environments. Is there any documentation for this case? Do we need to keep these pages up to date? Here are some links I found: https://docs.datastax.com/en/dse/6.8/dse-dev/datastax_enterprise/config/cassandraSystemProperties.html https://cassandra.apache.org/doc/4.1/cassandra/getting_started/configuring.html#environment-variables On Thu, 18 May 2023 at 16:03, Miklosovic, Stefan <stefan.mikloso...@netapp.com> wrote: > > Hi Maxim, > > thanks for bringing this up. I am glad you did the heavy-lifting in / around > CassandraRelevantProperties and we can build on top of this. > > I am fine with @Replaces for Cassandra system properties. After we put > everything into CassandraRelevantProperties, one can easily see that there > are great inconsistencies in properties' naming. As we still need to support > the old names too, using @Replaces, the similar mechanism we used in > DatabaseDescriptor, seems like the ideal solution. > > By the way, when somebody queries system_views.system_properties, it looks > very strange in CQL shell, the formatting is just broken. EXPAND ON; does not > help either. It is quite hard to parse this visually if a user wants to see > them all. The reason is that there is "java.class.path" property for which > the value is so long that it basically breaks the output. > > Another solution would be to fix the output but I am not sure how it would > look like. > > As we are going to rename them to have same prefixes, could not we remodel > that table as well? For example: > > https://gist.github.com/smiklosovic/de662b7faa25e1fdd56805cdb5ba80a7 > > Feel free to come up with a different approach. > > By doing this, it would be way easier to get just Cassandra properties or > just properties for tests or just Java properties and selecting just the > first two groups would not break CQLSH. It is nice that it would have same > prefix but I am trying to find a way how to utilize the same prefix in CQLSH > as well. > > ________________________________________ > From: Maxim Muzafarov <mmu...@apache.org> > Sent: Thursday, May 18, 2023 12:54 > To: dev@cassandra.apache.org > Subject: Re: [DISCUSS] Moving system property names to the > CassandraRelevantProperties > > NetApp Security WARNING: This is an external email. Do not click links or > open attachments unless you recognize the sender and know the content is safe. > > > > > Hello everyone, > > > Thanks for following this thread and the review, all the system > properties have been moved to CassandraRelevantProperties. > So you can find out what it looks like from the following link: > https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/config/CassandraRelevantProperties.java#L38 > > > I would like to show you a few more steps in this thread so that the > solution is generally complete. As you may have noticed, we have three > types of system properties: cassandra properties used in production > environments, cassandra properties used for testing only, and > non-cassandra properties. I would like to reuse the @Replaces > annotation to rename cassandra-related properties according to the > following pattern: the 'cassandra.' prefix is for production > properties, and the 'cassandra.test' prefix is for testing properties. > > This makes the results of the SystemPropertiesTable virtual table look > more natural to users. I thinks we should include this change for the > 5.0 release. > WDYT? > > > The other code clarity minor improvements to do: > > 1. > Use WithProperties to ensure that system properties are handled > https://issues.apache.org/jira/browse/CASSANDRA-18453 > > 2. > As a draft agreement, the CassandraRelevantProperties and > CassandraRelevantEnv (and probably DatabaseDescriptor) could share the > same interface to access the system properties, configuration > properties, and/or environment variables. The idea is still in draft > form, so I'm mentioning it here to keep it in context. Will come back > to it when more details are available. > This is what it might look like: > https://github.com/apache/cassandra/pull/2300/files#diff-6b7db8438314143a1b6b1c8c58901a4ebbbb3954af8cdd294ca8853a1001c1f4R70 > > On Fri, 31 Mar 2023 at 07:08, Jacek Lewandowski > <lewandowski.ja...@gmail.com> wrote: > > > > I'll do > > > > - - -- --- ----- -------- ------------- > > Jacek Lewandowski > > > > > > czw., 30 mar 2023 o 22:09 Miklosovic, Stefan <stefan.mikloso...@netapp.com> > > napisał(a): > >> > >> Hi list, > >> > >> we are looking for one more committer to take a look at this patch (1, 2). > >> > >> It looks like there is a lot to go through because of number of files > >> modified (around 200) but changes are really just about moving everything > >> to CassandraRelevantProperties. I do not think that it should take more > >> than 1 hour of dedicated effort and we are done! > >> > >> Thanks in advance to whoever reviews this. > >> > >> I want to especially thank Maxim for his perseverance in this matter and I > >> hope we will eventually deliver this work to trunk. > >> > >> (1) https://github.com/apache/cassandra/pull/2046 > >> (2) https://issues.apache.org/jira/browse/CASSANDRA-17797 > >> > >> Regards > >> > >> Regards > >> > >> ________________________________________ > >> From: Miklosovic, Stefan <stefan.mikloso...@netapp.com> > >> Sent: Wednesday, March 22, 2023 14:34 > >> To: dev@cassandra.apache.org > >> Subject: Re: [DISCUSS] Moving system property names to the > >> CassandraRelevantProperties > >> > >> NetApp Security WARNING: This is an external email. Do not click links or > >> open attachments unless you recognize the sender and know the content is > >> safe. > >> > >> > >> > >> > >> Hi Maxim, > >> > >> thanks for letting us know. > >> > >> I reviewed it couple months ago but I can revisit it to double check. We > >> need the second reviewer. Until we find somebody, we can not merge this. > >> > >> If anybody wants to take a look, it would be awesome. It seems like a lot > >> of changes / files touched but it is just about centralizing all > >> properties scattered around the code base into one place. > >> > >> Regards > >> > >> ________________________________________ > >> From: Maxim Muzafarov <mmu...@apache.org> > >> Sent: Tuesday, March 21, 2023 22:59 > >> To: dev@cassandra.apache.org > >> Subject: Re: [DISCUSS] Moving system property names to the > >> CassandraRelevantProperties > >> > >> NetApp Security WARNING: This is an external email. Do not click links or > >> open attachments unless you recognize the sender and know the content is > >> safe. > >> > >> > >> > >> > >> Hello everyone, > >> > >> This a friendly reminder that some help is still needed with the review :-) > >> I have resolved all the conflicts that have arisen in the last month or > >> two. > >> > >> If you'd like to invest some time in code clarity, please take a look: > >> https://github.com/apache/cassandra/pull/2046/files > >> > >> On Wed, 8 Feb 2023 at 19:48, Maxim Muzafarov <mmu...@apache.org> wrote: > >> > > >> > Hello everyone, > >> > > >> > > >> > We are trying to clean up the source code around the direct use of > >> > system properties and make this use more manageable and transparent. > >> > To achieve this, I have prepared a patch that moves all system > >> > property names to the CassandraRelevantProperties, which in turn makes > >> > some of the properties visible to a user through the > >> > SystemPropertiesTable virtual table. > >> > > >> > The patch has passed a few rounds of review, but we still need another > >> > pair of eyes to make sure we are not missing anything valuable. > >> > Please, take a look at the patch. > >> > > >> > You can find all the changes here: > >> > https://issues.apache.org/jira/browse/CASSANDRA-17797 > >> > > >> > > >> > I'd also like to share the names of the properties that will appear in > >> > the SystemPropertiesTable, the appearance of which is related to the > >> > public API changes we agreed to discuss on the dev list. > >> > > >> > > >> > The public API changes > >> > > >> > Newly production system properties added: > >> > > >> > io.netty.eventLoopThreads > >> > io.netty.transport.estimateSizeOnSubmit > >> > java.security.auth.login.config > >> > javax.rmi.ssl.client.enabledCipherSuites > >> > javax.rmi.ssl.client.enabledProtocols > >> > ssl.enable > >> > log4j2.disable.jmx > >> > log4j2.shutdownHookEnabled > >> > logback.configurationFile > >> > > >> > Newly added and used for tests only: > >> > > >> > invalid-legacy-sstable-root > >> > legacy-sstable-root > >> > org.apache.cassandra.tools.UtilALLOW_TOOL_REINIT_FOR_TEST > >> > org.caffinitas.ohc.segmentCount > >> > suitename > >> > sun.stderr.encoding > >> > sun.stdout.encoding > >> > test.bbfailhelper.enabled > >> > write_survey
