[GitHub] [accumulo] dlmarion commented on a diff in pull request #3329: Throw new exception when invalid tablet hosting request made

["dlmarion (via GitHub)" <gi...@apache.org>]

dlmarion commented on code in PR #3329:
URL: https://github.com/apache/accumulo/pull/3329#discussion_r1181537108


##########
core/src/main/java/org/apache/accumulo/core/client/admin/TableOperations.java:
##########
@@ -213,10 +214,12 @@ Collection<Text> listSplits(String tableName, int maxSplits)
    * @param ranges The input ranges that should be mapped to tablet servers and tablets.
    *
    * @throws TableOfflineException if the table is offline or goes offline during the operation
+   * @throws InvalidTabletHostingRequestException if tablet hosting is requested for table with
+   *         NEVER goal
    * @since 1.8.0
    */
-  Locations locate(String tableName, Collection<Range> ranges)
-      throws AccumuloException, AccumuloSecurityException, TableNotFoundException;

Review Comment:
   I applied your patch so that I could better see the differences. For the tablets that are returned, I don't think all of them have a goal of ALWAYS as the one's returned in the initial call to `locator.findTablets` could be currently hosted, but with a goal of ONDEMAND. The retry code checks that the tablets have a goal of ALWAYS. This change also clears the TabletLocator, so any other concurrent operation may incur a latency penalty when it tries to scan / write to a Tablet. Since this change introduces a call to Ample to check that the `locationless` are ALWAYS hosted, I wonder if `TableOperations.locate` should just call Ample and return the `Locations` for ALWAYS hosted Tablets for the requested table and not clear/use the TabletLocator.



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