snazy commented on code in PR #4545:
URL: https://github.com/apache/polaris/pull/4545#discussion_r3315580652
##########
persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/mutation/PolicyMutation.java:
##########
@@ -47,6 +49,20 @@ public record PolicyMutation(
long targetId,
boolean doAttach,
@NonNull Map<String, String> parameters) {
+ /**
+ * Conservative limit for serialized policy-mapping index value size to
ensure that the serialized
+ * index-entry value sizes don't grow too large.
+ *
+ * <p>If this limit is ever exceeded, there are two options:
+ *
+ * <ul>
+ * <li>If it's a marginal increase, it's possible to increase this value.
+ * <li>If the property bag is becoming too big, the policy mapping should
be stored in a
+ * separate new {@link Obj}ect.
+ * </ul>
+ */
+ public static final int MAX_POLICY_MAPPING_INDEX_VALUE_SIZE = 384;
Review Comment:
There isn't really a deeper policy-level reason for 384. The policy mapping
parameters are an opaque property bag here, and AFAICT the policy design does
not define what is supposed to go into it or what a reasonable size would be.
So this is mostly a NoSQL-storage guardrail. The mapping value is stored
inline in the index, and index values are serialized as-is. 384 is
intentionally conservative: large enough for the small parameter bags we
currently support/test, but small enough that one mapping cannot take a
noticeable chunk of the embedded index.
If we ever find a real use case that needs substantially more data in the
mapping, I don't think we should keep bumping this forever. Then the payload
should probably move into a separate object and the index should only keep the
lookup data / reference.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
