Re: [DISCUSS] Go version in go.mod (GoCQL)
I just noticed CASSGO-34 [1] was created for this topic and there was some discussion there. I posted a comment on the JIRA ticket to see if we can focus the discussion on this thread. Eric brought up in CASSGO-34 that we could keep the version on go.mod the actual required version to compile gocql and not necessarily the one that is officially supported because that would be beneficial for users that are not ready to upgrade Go on their critical production services right away. Personally I don't see an issue with using a Go version in go.mod that is lower than the versions the project officially supports but if we don't have any automated checks using that version then we might break compatibility with that version without being aware of it. Maybe this is ok since it's not officially supported? Adding another version to the test matrix kind of goes against what the project documents as officially supported Go versions, users might get confused when they look at the test matrix and see that there is a Go version there that is not officially supported. I'm +1 on keeping the Go version in go.mod the minimum version required to compile gocql and not necessarily keeping it in sync with the officially supported Go versions and I'm +0 on adding a "compile check" with that version in our CI to make sure we know right away when the version in go.mod needs to be updated. This doesn't mean we can't use features that require a higher version of Go, but we can be mindful and decide when it is worth doing that. [1] https://issues.apache.org/jira/browse/CASSGO-34 Jackson Fleming escreveu (terça, 19/11/2024 à(s) 22:11): > I personally think we should strive to align the go version in go.mod to > what we test against and officially support, it clearly describes what the > project is built and tested against to other developers/users, but happy to > be told that's overkill and 1.19 is fine. If there is a time to push to > newer go versions though, it's during a major release of the driver. > > If we are going to put 1.19 in the go.mod file - we should consider adding > some tests against it, to ensure compatibility doesn't get broken by > mistake? > > On Fri, Nov 8, 2024 at 3:13 AM João Reis wrote: > >> The Go version that is specified in "go.mod" is 1.13 but this is out of >> date, 1.17 is required to build GoCQL at the moment. Bumping the version on >> this file to 1.17 needs to be done regardless but should we bump this >> version to the minimum Go version that GoCQL officially supports or the one >> that is actually required to build? For context, GoCQL officially supports >> the latest 2 Go releases (currently 1.22 and 1.23) and these versions are >> what we use in CI. >> >> This topic was brought up while reviewing the PR [1] for CASSGO-1 (v5 >> support) [2] because a reference to "atomic.Bool" was added which requires >> Go 1.19. >> >> Personally I don't think we have to bump the version on go.mod to a >> version that we test against in CI but bumping it to 1.19 seems reasonable >> to me. >> >> Bumping the version on go.mod to a version that we use in CI would allow >> us to use new Go features sooner but it would also require every user to >> upgrade the Go version on their application. >> >> [1] https://github.com/apache/cassandra-gocql-driver/pull/1822 >> [2] https://issues.apache.org/jira/browse/CASSGO-1 >> >
Re: [DISCUSS] Delivery of CASSANDRA-19556 Add guardrail to block DDL/DCL queries
> 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
Re: [DISCUSS] Delivery of CASSANDRA-19556 Add guardrail to block DDL/DCL queries
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 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 > > >
[DISCUSS] Delivery of CASSANDRA-19556 Add guardrail to block DDL/DCL queries
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
Re: [DISCUSS] Delivery of CASSANDRA-19556 Add guardrail to block DDL/DCL queries
> 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 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=178800
Re: [DISCUSS] GoCQL sub modules vs packages
+1 on making the change before cutting a 2.0.0 release Jackson Fleming escreveu (terça, 19/11/2024 à(s) 22:03): > I concur with the approach of having a single module for the driver, I > don't think this project is complicated enough yet to require > separate modules, the project is new to the Apache foundation and keeping > things simple will make it easier for people to learn and contribute. > > I also agree that we need to look at moving lz4 from its own module into > the gocql module as a package. I think that it makes sense to do this to > remove the circular dependency, we really shouldn't ever be in the > situation where we have that happening, at best it'll look odd to people > paying attention, but it'll probably lead to headaches later on if we leave > it. Probably best to do that before cutting a 2.0.0 release? > > On Fri, Nov 8, 2024 at 2:55 AM João Reis wrote: > >> The goal of CASSGO-21 [1] is to move HostPoolHostPolicy out of the main >> package so that users don't have to download the >> github.com/hailocab/go-hostpool dependency if they are not using this >> specific policy. >> The question is whether we want to move this policy to a separate module >> or if it is enough to move it to a separate package on the same module. >> There is currently an open PR for this issue that uses the separate package >> approach [2]. >> >> The way we choose to resolve CASSGO-21 should dictate what we do in the >> future for similar "extension" like code. For example, the PR [4] for >> CASSGO-9 [3] adds 2 "extensions" that provide out of the box functionality >> to use the logging libraries "zap" and "zerolog" with gocql and these are >> implemented in their own independent modules that are located in an >> "extensions" folder of gocql's repository. >> >> Lz4 support is also implemented as an independent module at the moment >> and as part of CASSGO-1 (protocol v5 support) we need to run the >> integration test suite with lz4 compression since snappy is not supported >> in protocol v5 but this creates a circular dependency between >> "gocql/go.mod" and "gocql/lz4/go.mod". For context, CI only uses snappy >> compression on integration tests at the moment. The circular dependency >> might be ok since its usage in the "gocql" package is limited to "_test.go" >> files, but it doesn't look great. >> >> In summary, I think we should either move HostPoolHostPolicy to a new >> module or we should consider making "gocql/lz4" a package instead of a >> module and changing CASSGO-9's patch so that zap and zerolog support is >> also implemented in separate packages instead of modules. >> >> In my opinion we should use packages within the main "gocql" module for >> this instead of separate modules (at the very least for >> "HostPoolHostPolicy" and "lz4"). Separate modules could make sense when we >> want to independently version and release a particular "extension" but in >> my opinion I don't think we have that requirement for the specific use >> cases that I've mentioned so far. >> >> [1] https://issues.apache.org/jira/browse/CASSGO-21 >> [2] https://github.com/apache/cassandra-gocql-driver/pull/1770 >> [3] https://issues.apache.org/jira/browse/CASSGO-9 >> [4] https://github.com/apache/cassandra-gocql-driver/pull/1755 >> [5] https://issues.apache.org/jira/browse/CASSGO-1 >> >
Re: [DISCUSS] Delivery of CASSANDRA-19556 Add guardrail to block DDL/DCL queries
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 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 > 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
