yifan-c commented on code in PR #2251:
URL: https://github.com/apache/cassandra/pull/2251#discussion_r1156294284


##########
src/java/org/apache/cassandra/db/SSTableImporter.java:
##########
@@ -130,6 +131,8 @@ synchronized List<String> importNewSSTables(Options options)
             {
                 try
                 {
+                    if (abort)
+                        throw new InterruptedException("SSTables import has 
been aborted");

Review Comment:
   If the only reason for aborting is because of draining. Would using 
`operationMode` simplify the implementation? The changes in CFS and 
StorageService can be dropped. 
   
   ```
   if 
(StorageService.instance.getOperationMode().equalsIgnoreCase(StorageService.Mode.DRAINING.toString()))
   ``` 
   
   Another question is that is it the only place to abort the import? The 
sstables could be opening already before invoking `drain`. Those sstables won't 
be aborted. 
   It is not possible/clean to add the check everywhere. It might make sense to 
add one extra check before line#183 to avoid loading and building SI of the 
newly opened sstables. 



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

Reply via email to