[GitHub] keith-turner commented on a change in pull request #300: ACCUMULO-4708 Limit RFile block size to 2GB

[gi...@git.apache.org]

keith-turner commented on a change in pull request #300: ACCUMULO-4708 Limit RFile block size to 2GB
URL: https://github.com/apache/accumulo/pull/300#discussion_r139480306
 
 

 ##########
 File path: core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
 ##########
 @@ -426,6 +426,10 @@ public void append(Key key, Value value) throws IOException {
         throw new IllegalArgumentException("Keys appended out-of-order.  New key " + key + ", previous key " + prevKey);
       }
 
+      if (((long) key.getSize() + (long) value.getSize()) >= Integer.MAX_VALUE) {
 
 Review comment:
   Serialization will also add overhead. For example some bytes encoding the length of the row are written before the rows data.  So this means that what can really be written is less than what is checked for here.  Not sure if we want to try to account for that here.  We could do something simple like assume each field will need at least 5 bytes for the length (I think 5 is the max because we are using var int).  
   
   Even if not accounting for that serialization overhead this is still a really nice check.  It will catch most situations at the source leaving a small range of key+val size that would only be caught later when encoding the block.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services