dlmarion commented on code in PR #5743: URL: https://github.com/apache/accumulo/pull/5743#discussion_r2215952364
########## core/src/main/java/org/apache/accumulo/core/file/rfile/RFileOperations.java: ########## @@ -136,10 +137,45 @@ protected FileSKVWriter openWriter(FileOptions options) throws IOException { String file = options.getFilename(); FileSystem fs = options.getFileSystem(); - if (options.dropCacheBehind) { + var ecEnable = EcEnabled.valueOf( + options.getTableConfiguration().get(Property.TABLE_ENABLE_ERASURE_CODES).toUpperCase()); + + if (fs instanceof DistributedFileSystem) { + var builder = ((DistributedFileSystem) fs).createFile(new Path(file)).bufferSize(bufferSize) + .blockSize(block).overwrite(false); + + if (options.dropCacheBehind) { + builder = builder.syncBlock(); + } + + switch (ecEnable) { + case ENABLE: + String ecPolicyName = + options.getTableConfiguration().get(Property.TABLE_ERASURE_CODE_POLICY); + builder = builder.ecPolicyName(ecPolicyName); + break; + case DISABLE: + // force replication + builder = builder.replication((short) rep).replicate(); + break; + case DIR: Review Comment: I'm thinking we could get rid of this case, and also remove the `replicate()` call from the DISABLE case. ########## 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: Should we fail if TABLE_FILE_REPLICATION is set to a non-zero value? ########## core/src/main/java/org/apache/accumulo/core/conf/Property.java: ########## @@ -1412,6 +1412,19 @@ public enum Property { + "constraint.", "2.0.0"), + TABLE_ENABLE_ERASURE_CODES("table.file.ec", "dir", PropertyType.EC, Review Comment: I wonder if we need `DIR` for the user facing property. For whatever backing store is being used, there is a default behavior. For HDFS, it's replication. `enable` here is overriding the default behavior and opting-in to EC. `disable` here allows the user to change the value at runtime, but `disable` I think just means use the default behavior. -- 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