cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [cassandra] adelapena commented on a change in pull request #570: CASSANDRA-13606 Improve handling of 2i initialization failures
Date Mon, 18 May 2020 11:46:58 GMT

adelapena commented on a change in pull request #570:
URL: https://github.com/apache/cassandra/pull/570#discussion_r426566283



##########
File path: src/java/org/apache/cassandra/index/SecondaryIndexManager.java
##########
@@ -655,6 +695,9 @@ private synchronized void markIndexFailed(Index index)
                 SystemKeyspace.setIndexRemoved(baseCfs.keyspace.getName(), indexName);
 
             needsFullRebuild.add(indexName);
+
+            if (!index.getSupportedLoadTypeOnFailure().supportsWrites() && writableIndexes.remove(indexName)
!= null)
+                logger.info("Index [" + indexName + "] became not-writable.");

Review comment:
       > Is it reasonable a 'not ready' index should be serving reads or writes?
   
   For initial build it should't serve reads, but for rebuilds it's more complicated because
the index could still be in good condition. IMO if we were creating the index API from scratch
or adding a new implementation probably the best option would be to set not properly (re)built
index as unable to serve reads and writes. But, given that we have had the cf-based implementation
around for a while, I think we should by now preserve their original behaviour and provide
the API mechanisms to change that in new implementations and perhaps in the standard indexes
in the future. That and the recovery task seems a pair of nice improvements to ship with this
ticket.
   
   Accepting reads and writes when failed makes some sense in the particular case of cf-based
regular indexes because they don't have an initialization task and they are based on idempotent
operations on the underlaying column family. I'd say they focus in availability over consistency
because even failed rebuilds always leave the index is same or better condition than before.
In contrast, it's easy to imagine other implementations where a failed rebuild leaves the
index completely corrupt and unusable. I think there could easily be use cases out there relying
on this behaviour, and I'm afraid of arbitrarily changing that behaviour without coming back.
If we still want to move to the new approach, either in this ticket or in a dedicated one,
we should probably open a discussion in the mail list to see if there are deployments relaying
on the classic behaviour. 
   
   Another tempting option to ease the migration of cf-based indexes to the new consistency-over-availability
approach would be making the enum returned by `getSupportedLoadTypeOnFailure` configurable
through index options, for example:
   ```
   CREATE INDEX my_idx ON t WITH OPTIONS = {'supported_load_on_failed_init': 'WRITE', 'supported_load_on_failed_rebuild':
'ALL'}
   ```
   This is something that perhaps we could do in the ticket to change the default behaviour
of regular cf-based indexes, while keeping this ticket focused on the API changes and the
recovery task.
   
   WDYT? Maybe I'm wrong and there isn't anyone out there relying on the old 2i behaviour
and we should go straight to the the new failed-rebuild-means-broken behaviour.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Mime
View raw message