dlmarion commented on code in PR #5478:
URL: https://github.com/apache/accumulo/pull/5478#discussion_r2046909571
##########
core/src/main/java/org/apache/accumulo/core/file/FilePrefix.java:
##########
@@ -18,25 +18,90 @@
*/
package org.apache.accumulo.core.file;
-import java.util.stream.Stream;
+import java.util.EnumSet;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
public enum FilePrefix {
- BULK_IMPORT("I"), MINOR_COMPACTION("F"), MAJOR_COMPACTION("C"),
MAJOR_COMPACTION_ALL_FILES("A");
+ ALL("*"),
+ MINOR_COMPACTION("F"),
+ BULK_IMPORT("I"),
+ MAJOR_COMPACTION("C"),
+ MAJOR_COMPACTION_ALL_FILES("A"),
+ MERGING_MINOR_COMPACTION("M");
final String prefix;
FilePrefix(String prefix) {
this.prefix = prefix;
}
+ public String toPrefix() {
+ return this.prefix;
+ }
+
+ public String createFileName(String fileName) {
+ Objects.requireNonNull(fileName, "filename must be supplied");
+ Preconditions.checkArgument(!fileName.isBlank(), "Empty filename
supplied");
+ if (this == ALL || this == MERGING_MINOR_COMPACTION) {
+ throw new IllegalStateException(
+ "Unable to create filename with ALL, MERGING_MINOR_COMPACTION, or
UNKNOWN prefix");
+ }
+ return prefix + fileName;
+ }
+
public static FilePrefix fromPrefix(String prefix) {
- return Stream.of(FilePrefix.values()).filter(p ->
p.prefix.equals(prefix)).findAny()
- .orElseThrow(() -> new IllegalArgumentException("Unknown prefix type:
" + prefix));
+ Objects.requireNonNull(prefix, "prefix must be supplied");
+ Preconditions.checkArgument(!prefix.isBlank(), "Empty prefix supplied");
+ Preconditions.checkArgument(prefix.length() == 1, "Invalid prefix
supplied: " + prefix);
+ switch (prefix.toUpperCase()) {
+ case "A":
+ return MAJOR_COMPACTION_ALL_FILES;
+ case "C":
+ return MAJOR_COMPACTION;
+ case "F":
+ return MINOR_COMPACTION;
+ case "I":
+ return BULK_IMPORT;
+ case "M":
+ return MERGING_MINOR_COMPACTION;
+ default:
+ throw new IllegalArgumentException("Unknown prefix type: " + prefix);
Review Comment:
> I don't think the fixed-size stream over the enum values array is
performance-critical, and should be fine.
If I had my way, I would change all the occurrences of streams in the entire
code base back to for-loops. While the streams api may be more intuitive for
some, everyone knows what a for loop is. We are choosing to use an api that we
know is slower. If we were developing a calendar app, I would agree that
performance is not critical. For a database, I think performance is critical
and should be a design goal. On a lightly loaded system the implementation of a
non-critical section of code doesn't matter. On a highly loaded system, the
implementation of a non-critical section of code is preventing other code from
running.
--
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]
