maedhroz commented on code in PR #2492:
URL: https://github.com/apache/cassandra/pull/2492#discussion_r1264354194
##########
src/java/org/apache/cassandra/index/sai/disk/StorageAttachedIndexWriter.java:
##########
@@ -209,13 +204,11 @@ public void abort(Throwable accumulator)
*/
public void abort(Throwable accumulator, boolean fromIndex)
{
+ if (aborted) return;
+
// Mark the write operation aborted, so we can short-circuit any
further operations on the component writers.
aborted = true;
- // Make any indexes involved in this transaction non-queryable, as
they will likely not match the backing table.
- if (fromIndex)
- indexes.forEach(StorageAttachedIndex::makeIndexNonQueryable);
-
Review Comment:
The changes in `abort()` here are probably what require some real
discussion. The position I'm going to try to defend here is that making the
index non-queryable is sub-optimal in all cases if our goal is to keep indexes
and backing tables consistent w/ one another. (I've fixed
`SegmentFlushingFailureTester` to reflect this, but more tests are probably
necessary around Memtable flush...not hard to do.)
(I don't see any problem w/ continuing to not throw when the abort is
upstream is generated from outside the index itself.)
For this to work, the companion changes in `SSTableWriter` are here:
https://github.com/apache/cassandra/pull/2492/files#diff-27996cac323bd3bafcfd28e84caf649d44f3b4d83477874ae4d08c7513303209R234
@adelapena @mike-tr-adamson
--
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]
