[GitHub] [accumulo] milleruntime commented on a change in pull request #1637: #1618 Investigate TODO in exception message in new Bulk Import

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

milleruntime commented on a change in pull request #1637:
URL: https://github.com/apache/accumulo/pull/1637#discussion_r442983640



##########
File path: core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java
##########
@@ -616,7 +616,9 @@ private Text toText(byte[] row) {
         if (containsPrevRow && containsEndRow) {
           mappings.get(ke).merge(mappings.remove(oke));
         } else {
-          throw new RuntimeException("TODO handle merges");
+          throw new RuntimeException("Unable to execute merge. Key extent "
+                  + oke.toString() + " extends beyond the range of Key extent "
+                  + ke.toString() + ". Out of range data would be lost.");

Review comment:
       This wording may be confusing to users.  Even though we are merging keys, it is happening mostly behind the scenes during a bulk import.  Perhaps something like:
   ```suggestion
             throw new RuntimeException("Error during bulk import: Unable to merge overlapping keys during a table split.  Key extent " + oke.toString() + " extends beyond the range of " + ke.toString());
   ```
   @keith-turner does this message make sense in the context of this method?




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