josh-mckenzie commented on pull request #1213: URL: https://github.com/apache/cassandra/pull/1213#issuecomment-953089175
Ok @azotcsit and @sumanth-pasupuleti, I spoke to @clohfink yesterday a bit and from a workflow perspective, if we could have the JMX call to denylist a key be a "hit it once and it adds and refreshes" that'd be ideal (reasonably so). We had that in an interim commit but removed it due to trying to keep the cache consistent which we later backed off on, so there's no reason we can't, and shouldn't, add that back in. First commit is adding that functionality back in and adjusting tests to deal with it, second is updating the documentation to reflect workflow for CQL (bulk) or JMX (single). The last outstanding thing I can't think of a good solution to is what to do if a node is gc thrashing, read and write CQL stages are a mess, and denylisting requires CQL mutation to work. While we *could* append a new deny listed key to the DenylistEntry's in the cache independently of, or before, the CQL mutation that adds it to the dedicated store for the denylist, that means we'll likely have a disjoint between what's deny listed in our cache vs. what's in CQL and makes the in-memory cache the authority vs. what's in the DB. I'm actually somewhat receptive to that, since the JMX call queries what's in the cache and actively denylisting, but I think we should probably leave that up to a follow-up ticket rather than trying to tackle that on the tail end of this... marathon. 😀 If you two are good with these last two commits, I think we're good to run the full suite of tests and merge this thing. @azotcsit to your earlier point about having another committer's eyes on, as this is an upstream of something that was reviewed by a couple committers already and you and Sumanth are both committers, AND this is disabled by default, I'm pretty comfortable with us merging if we're all good after these couple of commits and a test run. Thanks everyone for all the effort on this thing; it's been quite a haul polishing things up. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
