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. >>> >>> >>> >>>