Fix bug in DefaultKeySizeConstraint

[milleruntime]

• Stop DefaultKeySizeConstraint from allowing keys with a large rowId that
  did not have any column update mutations
• Add a test to DefaultKeySizeConstraintTest for this condition

[keith-turner]

The limited documentation on the class says it limits the size of keys. A column update would correspond to a key, so the existing code may correct. One change would be that it could break out of the loop after adding the first violation.

[milleruntime]

I wasn't sure but to me the row ID is an integral part of the Key and a user could very easily create a key larger than the constraint if they have a large row ID

[keith-turner]

a user could very easily create a key larger than the constraint if they have a large row ID

The row length was being factored into the size of each column update which would equate to the eventual key size.

[milleruntime]

Yes but if there aren't any Column updates the row length will be ignored. Adding this chunk of code to the test will cause it to fail now:

m = new Mutation(oversized);
assertEquals(Collections.singletonList(DefaultKeySizeConstraint.MAX__KEY_SIZE_EXCEEDED_VIOLATION), 
        constraint.check(null, m));

[milleruntime]

The BatchWriter at least prevents this from every happening and will throw an exception:

accumulo/core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java

Line 247 in 675e41a

throw new IllegalArgumentException("Can not add empty mutations");

[keith-turner]

Yes but if there aren't any Column updates the row length will be ignored.

Ok I see what you are thinking about. Hopefully that case does not matter. When a mutation has no column updates, the batchwriter should reject it. Also if a mutation w/ no column update were to make it through somehow, when its inserted into the in memory map its converted to keys/values. If column updates are empty, then no key values will be created.

[ctubbsii]

The current code seems to impose a limit on any total key size (4 dimensions: row + cf + cq + cv; doesn't include timestamp or internal delete flag dimensions). Your change seems to impose a limit on the total size of this mutation, and seems to turn it into a MutationSizeConstraint.

[milleruntime]

I guess it doesn't really matter if it is a bug or not, the question should be whether or not this should be changed. Changing this Constraint could break existing tables with big Row IDs.

[milleruntime]

Closing this since it looks like it there are checks to prevent this situation from occurring.