[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3091: Added code and tests to confirm ScanServers can scan offline tables

[GitBox <gi...@apache.org>]

ctubbsii commented on code in PR #3091:
URL: https://github.com/apache/accumulo/pull/3091#discussion_r1035123963


##########
core/src/main/thrift/tabletserver.thrift:
##########
@@ -265,6 +265,8 @@ service TabletScanClientService {
     1:client.ThriftSecurityException sec
     2:TSampleNotPresentException tsnpe
     3:ScanServerBusyException ssbe
+    4:NotServingTabletException nste
+    5:TooManyFilesException tmfe    

Review Comment:
   One of the expectations in a patch release is that we can do rolling upgrades, and clients and servers may be upgraded at different times because patch releases are both forwards and backwards compatible. These are both already TExceptions, which should be caught in the generated code as a generic catch-all. However, I'm not sure what the exact behavior would be when these are declared explicitly in 2.1.1, but not in 2.1.0 if a client and server of different versions are talking to each other.
   
   Before approving this, I would like to know that the behavior is sensible if there is a 2.1.0 client talking to a 2.1.1 server, or a 2.1.1 client talking to a 2.1.0 server when these exceptions are thrown. Have you tested these scenarios? What is the impact?



-- 
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: notifications-unsubscribe@accumulo.apache.org

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