ctubbsii commented on code in PR #3083:
URL: https://github.com/apache/accumulo/pull/3083#discussion_r1030027932
##########
core/src/main/java/org/apache/accumulo/core/client/admin/compaction/CompactionConfigurer.java:
##########
@@ -76,4 +86,20 @@ public Map<String,String> getOverrides() {
}
Overrides override(InputParameters params);
+
+ /**
+ *
+ * @param tablePropertiesWithOverrides
+ * table properties with overrides
+ * @return true if we should call setDropBehind on majc output file
+ *
+ * @since 2.1.1
+ */
+ static boolean
+ dropCacheBehindMajcOutput(Iterable<Entry<String,String>>
tablePropertiesWithOverrides) {
+ ConfigurationCopy cc = new ConfigurationCopy(tablePropertiesWithOverrides);
+ Map<String,String> customProps =
+ cc.getAllPropertiesWithPrefix(Property.TABLE_ARBITRARY_PROP_PREFIX);
+ return
Boolean.valueOf(customProps.getOrDefault(TABLE_MAJC_OUTPUT_DROP_CACHE,
"false"));
+ }
Review Comment:
This method is a public API addition and shouldn't be added in a patch
release. Beyond that, I don't quite understand:
1. the use of a static method in the interface (what's the relationship to
the interface? why does it live here instead of some internal util class?)
2. the use of a static method in the public API and how users would use this
method for their code (there's no javadoc comment, so it's not clear what its
purpose is)
3. it also seems a little inefficient to make a copy of the props in
ConfigurationCopy, make another copy of the subset of props that have the
custom table prefix, then get the property. Why not just iterate over the props
and match on the exact value of the property you're looking for, without
storing any intermediate copies?
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java:
##########
@@ -212,9 +216,21 @@ public CompactionStats call() throws IOException,
CompactionCanceledException {
try {
FileOperations fileFactory = FileOperations.getInstance();
FileSystem ns = this.fs.getFileSystemByPath(outputFile.getPath());
- mfw = fileFactory.newWriterBuilder()
+
+ boolean dropCacheBehindMajcOutput = false;
+ if (!RootTable.ID.equals(this.extent.tableId())
+ && !MetadataTable.ID.equals(this.extent.tableId())
+ && CompactionConfigurer.dropCacheBehindMajcOutput(acuTableConf)) {
+ dropCacheBehindMajcOutput = true;
+ }
Review Comment:
This is more concise.
```suggestion
boolean dropCacheBehindMajcOutput =
!RootTable.ID.equals(this.extent.tableId())
&& !MetadataTable.ID.equals(this.extent.tableId())
&& CompactionConfigurer.dropCacheBehindMajcOutput(acuTableConf);
```
--
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]
