yifan-c commented on code in PR #3355:
URL: https://github.com/apache/cassandra/pull/3355#discussion_r1949628834
##########
src/java/org/apache/cassandra/cql3/terms/Lists.java:
##########
@@ -414,12 +414,14 @@ static void doAppend(Term.Terminal value, ColumnMetadata
column, UpdateParameter
dataSize += cell.dataSize();
}
Guardrails.collectionSize.guard(dataSize,
column.name.toString(), false, params.clientState);
+ Guardrails.collectionListSize.guard(dataSize,
column.name.toString(), false, params.clientState);
Review Comment:
Let's go a bit deeper and compare the difference. (Yes. They have difference
behavior).
The curried function (my suggestion) defines the evaluation priority and
check once. That is, as long as collectionListSize is defined, it takes the
precedence, regardless of their threshold values. It makes sense, since this
patch brings a set of more finer-grained guardrails regarding collection sizes.
The handling in this patch, check the size _twice_, if they are both
defined/enabled. In this case, it is the lowest threshold value that enforce
the guardrail. The value could come from `collectionListSize` or
`collectionSize`. Imagining from users' perspective, if the size guardrail on
List is defined, it could cause confusion if a list size that satisfies the
list size guardrail fails (due to the lower threshold from collection size).
In the end, it needs documentation to elaborate the behaviors of the
overlapping guardrails.
--
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.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
