[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #904: PHOENIX-6167 Adding maxMutationCellSizeBytes config and exception

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

ChinmaySKulkarni commented on a change in pull request #904:
URL: https://github.com/apache/phoenix/pull/904#discussion_r497719456



##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
##########
@@ -139,6 +140,12 @@ private static void setValues(byte[][] values, int[] pkSlotIndex, int[] columnIn
         for (int i = 0, j = numSplColumns; j < values.length; j++, i++) {
             byte[] value = values[j];
             PColumn column = table.getColumns().get(columnIndexes[i]);
+            if (value.length >= maxCellSizeBytes) {

Review comment:
       What about use-cases that use SINGLE_CELL_ARRAY_WITH_OFFSETS storage scheme wherein all the column keyvalues are stored inside a single cell? In that case, the hbase cell limit would apply to the sum of all the columns (or something along those lines). Either we should expand this to apply to all storage schemes, or restrict scope to only apply to ONE_CELL_PER_COLUMN

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
##########
@@ -187,6 +213,9 @@ public static MutationState upsertSelect(StatementContext childContext, TableRef
         int maxSizeBytes =
                 services.getProps().getInt(QueryServices.MAX_MUTATION_SIZE_BYTES_ATTRIB,
                     QueryServicesOptions.DEFAULT_MAX_MUTATION_SIZE_BYTES);
+        int maxCellSizeBytes =

Review comment:
       I don't understand why we need another config for this. Introducing a Phoenix level config to mean the same thing as an existing hbase-level config will lead to confusion and now we have to make sure that this config is updated in case the main hbase config is updated. Why not just use 
   `hbase.client.keyvalue.maxsize` or `hbase.server.keyvalue.maxsize` ?




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