keith-turner commented on code in PR #5743:
URL: https://github.com/apache/accumulo/pull/5743#discussion_r2219779837
##########
server/base/src/main/java/org/apache/accumulo/server/util/PropUtil.java:
##########
@@ -74,7 +77,28 @@ protected static void validateProperties(final ServerContext
context,
"Unable to resolve classloader for context: " + prop.getValue());
}
}
+
+ if (prop.getKey().equals(Property.TABLE_ERASURE_CODE_POLICY.getKey())) {
Review Comment:
Possibly, not sure about a few things. For the case of compactions setting
replication we may want latent settings on the table. Like if the table has
erasure coding enabled but a compaction disables it for its output may want to
use the replication setting on the table for the file created by the
compaction. If we do not want to support this, then the compaction could also
specify the replication setting.
Looking at implementing this it does not seem like a simple change. To
validate an existing setting w/ a new setting need to consider a few cases.
Does the set of properties passed in contain the other prop, if not then read
it from the existing settings in ZK. In the case of a conflict would also need
to consider the version which validateProperties does not currently consider.
If there is a version mismatch want to prioritize throwing a concurrent
modification exception vs a validation exception. Also if this type of
validaiton is done then it needs to be bidirectional, do the check when setting
replication or erasure coding. If we want to do this check I think would be
best in a follow on PR that can focus on all these issues.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
