keith-turner closed pull request #373: ACCUMULO-4709 sanity check in Mutation
URL: https://github.com/apache/accumulo/pull/373
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java 
b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
index ebc72f5e22..5e1a7ba869 100644
--- a/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
+++ b/core/src/main/java/org/apache/accumulo/core/data/Mutation.java
@@ -37,6 +37,9 @@
 import org.apache.hadoop.io.Writable;
 import org.apache.hadoop.io.WritableUtils;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+
 /**
  * Mutation represents an action that manipulates a row in a table. A mutation 
holds a list of column/value pairs that represent an atomic set of modifications
  * to make to a row.
@@ -66,6 +69,13 @@
    */
   static final int VALUE_SIZE_COPY_CUTOFF = 1 << 15;
 
+  /**
+   * Maximum size of a mutation (2GB).
+   */
+  static final long MAX_MUTATION_SIZE = (1L << 31);
+
+  static final long SERIALIZATION_OVERHEAD = 5;
+
   /**
    * Formats available for serializing Mutations. The formats are described in 
a <a href="doc-files/mutation-serialization.html">separate document</a>.
    */
@@ -79,6 +89,10 @@
   private int entries;
   private List<byte[]> values;
 
+  // tracks estimated size of row.length + largeValues.length
+  @VisibleForTesting
+  long estRowAndLargeValSize = 0;
+
   private UnsynchronizedBuffer.Writer buffer;
 
   private List<ColumnUpdate> updates;
@@ -169,6 +183,7 @@ public Mutation(byte[] row, int start, int length, int 
initialBufferSize) {
     this.row = new byte[length];
     System.arraycopy(row, start, this.row, 0, length);
     buffer = new UnsynchronizedBuffer.Writer(initialBufferSize);
+    estRowAndLargeValSize = length + SERIALIZATION_OVERHEAD;
   }
 
   /**
@@ -306,6 +321,9 @@ private void put(byte[] cf, int cfLength, byte[] cq, int 
cqLength, byte[] cv, bo
     if (buffer == null) {
       throw new IllegalStateException("Can not add to mutation after 
serializing it");
     }
+    long estimatedSizeAfterPut = estRowAndLargeValSize + buffer.size() + 
cfLength + cqLength + cv.length + (hasts ? 8 : 0) + valLength + 2
+        + 4 * SERIALIZATION_OVERHEAD;
+    Preconditions.checkArgument(estimatedSizeAfterPut < MAX_MUTATION_SIZE && 
estimatedSizeAfterPut >= 0, "Maximum mutation size must be less than 2GB ");
     put(cf, cfLength);
     put(cq, cqLength);
     put(cv);
@@ -325,6 +343,7 @@ private void put(byte[] cf, int cfLength, byte[] cq, int 
cqLength, byte[] cv, bo
       System.arraycopy(val, 0, copy, 0, valLength);
       values.add(copy);
       put(-1 * values.size());
+      estRowAndLargeValSize += valLength + SERIALIZATION_OVERHEAD;
     }
 
     entries++;
diff --git 
a/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java 
b/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
index f58737babe..89671d1037 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
@@ -162,6 +162,10 @@ public void writeVLong(long i) {
         data[offset++] = (byte) ((i & mask) >> shiftbits);
       }
     }
+
+    public int size() {
+      return offset;
+    }
   }
 
   /**
diff --git a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java 
b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
index 7d247f5f4b..7f4e2fda75 100644
--- a/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/data/MutationTest.java
@@ -680,4 +680,12 @@ public void testPutAfterEquals() {
       fail("Calling Mutation#equals then Mutation#put should not result in an 
IllegalStateException.");
     }
   }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testSanityCheck() {
+    Mutation m = new Mutation("too big mutation");
+    m.put("cf", "cq1", "v");
+    m.estRowAndLargeValSize += (Long.MAX_VALUE / 2);
+    m.put("cf", "cq2", "v");
+  }
 }


 

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to