[GitHub] [accumulo] ctubbsii commented on a change in pull request #1752: New unload thrift endpoint. Closes #1750

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

ctubbsii commented on a change in pull request #1752:
URL: https://github.com/apache/accumulo/pull/1752#discussion_r511171869



##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftClientHandler.java
##########
@@ -1454,9 +1454,16 @@ public void run() {
     }
   }
 
+  @Deprecated
   @Override
   public void unloadTablet(TInfo tinfo, TCredentials credentials, String lock, TKeyExtent textent,
       TUnloadTabletGoal goal, long requestTime) {
+

Review comment:
       For RPC compatibility for rolling restarts, you should probably have a real implementation here, by calling the new unload method with the toString version of the goal.

##########
File path: core/src/main/thrift/tabletserver.thrift
##########
@@ -348,6 +350,16 @@ service TabletClientService extends client.ClientService {
     7:i64 requestTime
   )
 
+  // since 2.1
+  oneway void unload(
+      5:trace.TInfo tinfo
+      1:security.TCredentials credentials
+      4:string lock
+      2:data.TKeyExtent extent
+      6:string goal
+      7:i64 requestTime

Review comment:
       These fields could be numbered sanely, if you're creating a new function. The old fields were numbered that way because it evolved over time.




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