OK thank you Josh. Nobody seems to object system property hence property into jvm-server.option it will be.
Regards On Wed, Nov 20, 2024 at 4:08 PM Štefan Miklošovič <smikloso...@apache.org> wrote: > These are all good questions ... > > BTW technically it would not be in cassandra.yaml. I understand system > property to be a property you set to JVM upon it' start. It is not a > configuration property in yaml - It might go to jvm-server.options. > > I would say that if that system property is set then it overrides anything > in guardrails. Or in other words, if the property is specified (no schema > modifications possible), than guardrails would not be even checked. This > seems to be aligned with your line of thinking. > > But, consider this scenario: > > 1. operator sets property so no schema updates are possible > 2. rolling restart of a cluster (or might be set upon upgrading to a patch > release the system property will be in) > 3. operator starts to upgrade nodes to next major, one by one, but > properties are in jvm-server.options and that will stay (right?). So if the > configuration will not change in this regard, then after the rolling > upgrade, we will not be able to control that by newly introduced guardrails > because system property would have higher priority. > 4. one more rolling restart after the upgrade is necessary to turn off > system property so it is controllable by a guardrail. > > If we disabled the property upon upgrade, then there might be some nodes > which are upgraded to use guardrails and schema modifications would be > possible for these nodes, even not all nodes are upgraded yet! That is said > if we do not disable schema modifications via guardrail for just upgraded > nodes and we disable the system property. > > > > On Wed, Nov 20, 2024 at 3:43 PM Josh McKenzie <jmcken...@apache.org> > wrote: > >> What do you think should happen with this newly-introduced system >> property in trunk? Removing it mercilessly in the trunk without any >> deprecation? >> >> Is there a risk to leaving it in? I get the added complexity of having >> yet another system property in that insane .yaml file, but having multiple >> ways to configure the same thing seems pretty innocuous (if annoying) to me. >> >> Does open up the question though - should the system property be a hard >> override that JMX and/or guardrails aren't allowed to override, or should >> it just represent the default state of the restriction on node startup but >> you can then toggle it off? >> >> I intuitively lean to "if it's disabled in the .yaml, you have to bounce >> the nodes w/that flipped to disable it; dynamic override doesn't work". >> That'd give us the two usage patterns: >> >> 1. You have an upgrade coming and want to disable DDL temporarily for >> the upgrade window >> 2. You have schemas setup the way you want them and want to lock down >> a cluster completely to prevent accidental DDL mishaps >> >> >> >> On Wed, Nov 20, 2024, at 7:16 AM, Štefan Miklošovič wrote: >> >> Happy to leave the dynamic setting of this in runtime out and go with a >> good old system property in each patch release from 4.0.x to 5.0.x if it is >> enough for people. We would deprecate alter_table_enabled in 5.0.x and >> remove it in trunk with the introduction of CASSANDRA-19556. >> >> What do you think should happen with this newly-introduced system >> property in trunk? Removing it mercilessly in the trunk without any >> deprecation? >> >> On Wed, Nov 20, 2024 at 12:57 PM Josh McKenzie <jmcken...@apache.org> >> wrote: >> >> >> they have to turn off a node, set the property and turn it back on. This >> is not optimal >> >> We have quite a few other features that have this same paradigm to >> disable/enable them. Is there a reason it's worse in this case than those, >> at least on the 4.0 release? Users will have to bounce up to the newer >> patch release with this flag in it anyway so they *could* set this flag >> as part of that upgrade process if they were bumping up to just get this >> functionality pre-upgrade. At least in my experience rolling restarts >> aren't a huge ordeal. >> >> It seems like the real thorny part about all this is the introduction of >> the dynamic functionality back to 4.0 as it's a pretty straightforward >> deprecation path and UX for 5.1+ (i.e. deprecate old feature + add new >> guardrail + add JMX -> guardrail). >> >> On Wed, Nov 20, 2024, at 4:48 AM, Štefan Miklošovič wrote: >> >> Hello, >> >> I am having a little bit of a hard time delivering (1). Long story short, >> this was aimed to be added to 5.0 but we postponed it because it was too >> late so the new target is 5.1. >> >> What we planned to do is to (2) deprecate alter_table_enabled in 5.0.1 >> which is superseded by CASSANDRA-19556, then to introduce a system property >> to avoid schema modifications in 4.0, 4.1 and 5.0 branches and eventually >> replace alter_table_enabled by CASSANDRA-19556 which deals with this more >> robustly. >> >> The problem we seem to hit is that "system property" in 4.0 -> 5.0 is not >> enough, because if you think about that, system property has to be set when >> nodes start. So when operators want to prevent schema modification, they >> have to turn off a node, set the property and turn it back on. This is not >> optimal and we were thinking that introducing _something dynamic_ which can >> be set in runtime would be preferable, but we seem to hit a dead-end (3) >> because I am not sure what path to choose here and people have become >> unresponsive since. >> >> From my point of view, the most ideal outcome would be to introduce an >> mbean method to e.g. StorageService which sets a flag whether schema >> modifications are possible or not, we would ship this to 4.0 -> trunk. Then >> in 5.0 we would deprecate alter_table_enabled, in trunk we would remove >> alter_table_enabled and introduced CASSANDRA-19556 and finally, in trunk, >> the introduced MBean method on StorageService would internally translate to >> calling respective guardrails we just introduced (4) so we do not need to >> deprecate it yet again. >> >> Does this make sense to people? My biggest concern is about introducing >> MBean method to StorageService for lower branches as that technically >> counts as a new feature but I think that if we want to have this >> dynamically settable / in runtime, there is no way around that. >> >> I am all ears about doing this differently if you think what I propose >> here is just too much and might be done in an alternative way. >> >> (1) https://issues.apache.org/jira/browse/CASSANDRA-19556 >> (2) >> https://issues.apache.org/jira/browse/CASSANDRA-19556?focusedCommentId=17848709&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17848709 >> (3) >> https://issues.apache.org/jira/browse/CASSANDRA-19828?focusedCommentId=17880018&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17880018 >> (4) https://github.com/apache/cassandra/pull/3275 >> >> Regards >> >> >> >>
