+1 to the "on by default" camp. > What comes to mind is how we brought down people clusters and made sstables > unreadable with the introduction of the chunk_length configuration in 1.0 I think a key difference here is that changing chunk length is something that materially changes behavior and expectations w/a coupled system, whereas switching crypto providers has the much smaller failure mode of "the implementations aren't binary compatible even though they're supposed to be, and are very heavily tested TO be".
Totally agree that a "surprise! it didn't load so now your nodes won't start" approach would be a Very Bad Experience for users. Falling back from ACCP and squawking about the lack might actually be nice to help folks where it doesn't load / work / etc know to look into it. It really makes a material difference. On Wed, Jul 26, 2023, at 4:02 PM, Jordan West wrote: > It sounds like some of the concerns have shifted then. I would like to better > understand the YAML one. Like Jeremiah said it may be a better topic for the > ticket. Would appreciate an example exception or error people are concerned > about. > > If the issue is the “fail fast” on start I’m sure we can find a solution > everyone accepts and move forward. > > If we are agreed “on by default” is the way to go that’s awesome! > > Jordan > > On Wed, Jul 26, 2023 at 12:59 Jeremiah Jordan <jeremiah.jor...@gmail.com> > wrote: >> I had a discussion with Mick on slack. His concern is not with enabling >> ACCP. His concern is around the testing of the new C* yaml config code >> which is included in the patch that is used to decide if ACCP should be >> enabled or not, and if startup should fail if it can’t be enabled. >> >> I agree. We should make sure that the new C* yaml config code is solid >> before we commit this patch, especially when it has the possibility of cause >> node startup to fail on purpose. But that should be a discussion for the >> ticket I think, not for this thread. >> >> So I think we are back to the original question. Should ACCP be used by >> default in trunk. From what I have seen I do not see anyone who is against >> that? >> >> -Jeremiah >> >> >> On Jul 26, 2023 at 2:53:02 PM, Jordan West <jorda...@gmail.com> wrote: >>> +1 Scott. And agreed all involved are looking out for the best interests of >>> C* users. And I appreciate those with concerns contributing to addressing >>> them. >>> >>> I’m all for making upgrades smooth bc I do them so often. A huge portion of >>> our 4.1 qualification is “will it break on upgrade”? Because of that I’m >>> confident in this patch and concerned about many other areas. I think it’s >>> commedable to want to reach a point where teams have the trust in the >>> community to have done that for them but that starts w better test coverage >>> and concrete evidence. >>> >>> Given all that, I think we should move forward w Ayushi’s proposal to make >>> it on by default. >>> >>> Jordan >>> >>> On Wed, Jul 26, 2023 at 12:14 C. Scott Andreas <sc...@paradoxica.net> wrote: >>>> I think these concerns are well-intended, but they feel rooted in >>>> uncertainty rather than in factual examples of areas where risk is >>>> present. I would appreciate elaboration on the specific areas of risk that >>>> folks imagine. >>>> >>>> I would encourage those who express skepticism to try the patch, and I >>>> endorse Ayushi's proposal to enable it by default. >>>> >>>> >>>> – Scott >>>> >>>>> On Jul 26, 2023, at 12:03 PM, "Miklosovic, Stefan" >>>>> <stefan.mikloso...@netapp.com> wrote: >>>>> >>>>> >>>>> We can make it opt-in, wait one major to see what bugs pop up and we >>>>> might do that opt-out eventually. We do not need to hurry up with this. I >>>>> understand everybody's expectations and excitement but it really boils >>>>> down to one line change in yaml. People who are so much after the >>>>> performance will be definitely aware of this knob to turn on to squeeze >>>>> even more perf ... >>>>> >>>>> I look around dtests Jeremiah mentioned but I would just moved on and >>>>> make it opt-in if we are not 100% persuaded about it _yet_. >>>>> >>>>> ________________________________________ >>>>> From: Mick Semb Wever <m...@apache.org> >>>>> Sent: Wednesday, July 26, 2023 20:48 >>>>> To: dev@cassandra.apache.org >>>>> Subject: Re: [DISCUSS] Using ACCP or tc-native by default >>>>> >>>>> 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. >>>>> >>>>> >>>>> >>>>> >>>>> What comes to mind is how we brought down people clusters and made >>>>> sstables unreadable with the introduction of the chunk_length >>>>> configuration in 1.0. It wasn't about how tested the compression >>>>> libraries were, but about the new configuration itself. Introducing >>>>> silent defaults has more surface area for bugs than introducing explicit >>>>> defaults that only apply to new clusters and are so opt-in for existing >>>>> clusters. >>>>> >>>>> >>>>> >>>>> On Wed, 26 Jul 2023 at 20:13, J. D. Jordan >>>>> <jeremiah.jor...@gmail.com<mailto:jeremiah.jor...@gmail.com>> wrote: >>>>> Enabling ssl for the upgrade dtests would cover this use case. If those >>>>> don’t currently exist I see no reason it won’t work so I would be fine >>>>> for someone to figure it out post merge if there is a concern. What JCE >>>>> provider you use should have no upgrade concerns. >>>>> >>>>> -Jeremiah >>>>> >>>>>> On Jul 26, 2023, at 1:07 PM, Miklosovic, Stefan >>>>>> <stefan.mikloso...@netapp.com<mailto:stefan.mikloso...@netapp.com>> >>>>>> wrote: >>>>>> >>>>>> Am I understanding it correctly that tests you are talking about are >>>>>> only required in case we make ACCP to be default provider? >>>>>> >>>>>> I can live with not making it default and still deliver it if tests are >>>>>> not required. I do not think that these kind of tests were required >>>>>> couple mails ago when opt-in was on the table. >>>>>> >>>>>> While I tend to agree with people here who seem to consider testing this >>>>>> scenario to be unnecessary exercise, I am afraid that I will not be able >>>>>> to deliver that as testing something like this is quite complicated >>>>>> matter. There is a lot of aspects which could be tested I can not even >>>>>> enumerate right now ... so I try to meet you somewhere in the middle. >>>>>> >>>>>> ________________________________________ >>>>>> From: Mick Semb Wever <m...@apache.org<mailto:m...@apache.org>> >>>>>> Sent: Wednesday, July 26, 2023 17:34 >>>>>> To: dev@cassandra.apache.org<mailto:dev@cassandra.apache.org> >>>>>> Subject: Re: [DISCUSS] Using ACCP or tc-native by default >>>>>> >>>>>> 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. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Can you say more about the shape of your concern? >>>>>> >>>>>> >>>>>> Integration testing where some nodes are running JCE and others accp, >>>>>> and various configurations that are and are not accp compatible/native. >>>>>> >>>>>> I'm not referring to (re-) unit testing accp or jce themselves, or >>>>>> matrix testing over them, but our commitment to always-on upgrades >>>>>> against all possible configurations that integrate. We've history with >>>>>> config changes breaking upgrades, for as simple as they are. >>>> >>>>
