I would say keep the old ones and add new ones to be compatible in addition. Now your catch led me to more thinking and I think throwing ConfigurationException deserves more attention. I noticed in DatabaseDescriptor there are setters that are leaking it to JMX since cassandra-3.0. I am not sure whether we can just change it to IllegalArgumentException which will be then also thrown on startup where they were ConfigurationExceptions before. Not sure what is the right course of action here to fix things without being considered breaking changes too as I see different people doing different things in our codebase. Looking into our own NodeTool we have two error codes - 1 that catches a few java and airlift exceptions and 2 for other errors. My guess is we aim to throw only exceptions that will lead to exit code 1, no? I am not sure. GetColumnIndexSize was added in trunk and I see it throws ConfigurationException, that was acknowledged in its test. Considering the setter was always doing it I guess this was just normal addition but It seems to me it is actually a bug… Now I can change it to throw IllegalArgumentException. Looking at the code I think different people probably have different understanding so I want to align us to ensure we don’t break tools by fixing or not stuff… In many setters we were actually not doing the same checks we do on Startup too… I consider this being a bug.
On Tue, 5 Apr 2022 at 18:21, David Capwell <dcapw...@gmail.com> wrote: > There are 2 places that expose non-standard java classes, so JMX only > works if and only if the JMX client also has Cassandra's jars, else > they will fail; the 2 examples are > > org.apache.cassandra.service.StorageServiceMBean#enableAuditLog throws > org.apache.cassandra.exceptions.ConfigurationException, removing that > exception does not break binary compatibility, but does break source > as javac will say catching it isn't allowed as it doesn't throw. If > you call the method without Cassandra jars the method will work > properly until a ConfigurationException is thrown, in which case a > ClassNotFoundException will be thrown, hiding the error message. > > org.apache.cassandra.db.ColumnFamilyStoreMBean#forceCompactionForTokenRange > takes a collection of > org.apache.cassandra.dht.Range<org.apache.cassandra.dht.Token>, since > this is an operation the only people who could call it are those with > the jars in the path. > > Given that both are not properties and require setting values, jmx get > object does not break, so the impact is limited to the callers of each > method. > > We have a few options to address: > > 1) live with it. Allow these cases to keep doing what they are doing, > but block new cases from popping up > 2) live with it, but expose new versions which do not break JMX > 3) remove ConfigurationException from > StorageServiceMBean#enableAuditLog and accept the source compatibility > issue > 4) remove or fix ColumnFamilyStoreMBean#forceCompactionForTokenRange > 5) #3 and #4 > > What do others think? In CASSANDRA-17527 I am adding a test to detect > these cases and block them, I am also fixing the JMX issues on trunk > that were not released, so this thread is only limited to the cases > that were actually released > > Thanks! >
