The question isn’t so much about reporting them (we should), it’s about the behavior of tombstone_warn_threshold and tombstone_failure_threshold. The patch changes the behavior to include the number of rows that are passed over due to the range tombstones. We’re interested in feedback on if it makes sense to change the current behavior. I’m a +.5 on the change, it makes sense to me, but am wondering if there’s a case we haven’t considered. At the very least we’d need a NEWS entry since it is a behavior change.
> On Dec 21, 2017, at 12:33 PM, DuyHai Doan <doanduy...@gmail.com> wrote: > > +1 to report range tombstones. This one is quite tricky indeed to track > > +1 to Mockito too, with the reserve that it should be used wisely > > On Thu, Dec 21, 2017 at 9:11 PM, Jon Haddad <j...@jonhaddad.com> wrote: > >> I had suggested to Alex we kick this discussion over to the ML because the >> change will have a significant impact on the behavior of Cassandra when >> doing reads with range tombstones that cover a lot of rows. The behavior >> now is a little weird, a single tombstone could shadow hundreds of >> thousands or even millions of rows, and the query would probably just time >> out. Personally, I’m in favor of the change in behavior of this patch but >> I wanted to get some more feedback before committing to it. Are there any >> objections to what Alex described? >> >> Regarding Mockito, I’ve been meaning to bring this up for a while, and I’m >> a solid +1 on introducing it to help with testing. In an ideal world we’d >> have no singletons and could test everything in isolation, but >> realistically that’s a multi year process and we just aren’t there. >> >> >>> On Dec 19, 2017, at 11:07 PM, Alexander Dejanovski < >> a...@thelastpickle.com> wrote: >>> >>> Hi folks, >>> >>> I'm working on CASSANDRA-8527 >>> <https://issues.apache.org/jira/browse/CASSANDRA-8527> and would need to >>> discuss a few things. >>> >>> The ticket makes it visible in tracing and metrics that rows shadowed by >>> range tombstones were scanned during reads. >>> Currently, scanned range tombstones aren't reported anywhere which hides >>> the cause of performance issues during reads when the users perform >> primary >>> key deletes. >>> As such, they do not count in the warn and failure thresholds. >>> >>> While the number of live rows and tombstone cells is counted in the >>> ReadCommand class, it is currently not possible to count the number of >>> range tombstones there as they are merged with the rows they shadow >> before >>> reaching the class. >>> Instead, we can count the number of deleted rows that were read , which >>> already improves diagnosis and show that range tombstones were scanned : >>> >>> if (row.hasLiveData(ReadCommand.this.nowInSec(), enforceStrictLiveness)) >>> ++liveRows; >>> else if (!row.primaryKeyLivenessInfo().isLive(ReadCommand.this. >> nowInSec())) >>> { >>> // We want to detect primary key deletions only. >>> // If all cells have expired they will count as tombstones. >>> ++deletedRows; >>> } >>> >>> Deleted rows would be part of the warning threshold so that we can spot >> the >>> range tombstone scans in the logs and tracing would look like this : >>> >>> WARN [ReadStage-2] 2017-12-18 18:22:31,352 ReadCommand.java:491 - >>> Read 2086 live rows, 2440 deleted rows and 0 tombstone cells for >>> query.. >>> >>> >>> Are there any caveats to that approach ? >>> Should we include the number of deleted rows in the failure threshold or >>> make it optional, knowing that it could make some queries fail while they >>> were passing before ? >>> >>> On a side note, is it ok to bring in Mockito in order to make it easier >>> writing tests ? I would like to use a Spy in order to write some tests >>> without starting the whole stack. >>> >>> Thanks, >>> >>> >>> -- >>> ----------------- >>> Alexander Dejanovski >>> France >>> @alexanderdeja >>> >>> Consultant >>> Apache Cassandra Consulting >>> http://www.thelastpickle.com >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org >> For additional commands, e-mail: dev-h...@cassandra.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org For additional commands, e-mail: dev-h...@cassandra.apache.org
