ctubbsii commented on code in PR #3321:
URL: https://github.com/apache/accumulo/pull/3321#discussion_r1175924473
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java:
##########
@@ -401,6 +401,23 @@ public static String getRowPrefix() {
}
+ /**
+ * Holds error message processing flags
+ */
+ public static class ProblemSection {
+ private static final Section section =
+ new Section(RESERVED_PREFIX + "err_", true, RESERVED_PREFIX + "err`",
false);
Review Comment:
This section definition includes the underscore as a mandatory part of the
section. That's the most narrowly scoped definition, and completely fine.
However, I'm a bit surprised that the section wasn't defined more widely as
everything starting with `err`, as in something like:
```java
new Section(RESERVED_PREFIX + "err", true, RESERVED_PREFIX + "ers",
false);
```
I am not suggesting changing it... just noting the distinction between the
narrow section definition and the wider definition.
##########
server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReports.java:
##########
@@ -217,9 +218,9 @@ private Iterator<Entry<Key,Value>> getIter2() {
scanner.setTimeout(3, TimeUnit.SECONDS);
if (table == null) {
- scanner.setRange(new Range(new Text("~err_"), false, new
Text("~err`"), false));
+ scanner.setRange(ProblemSection.getRange());
} else {
- scanner.setRange(new Range(new Text("~err_" + table)));
+ scanner.setRange(new Range(new
Text(ProblemSection.getRowPrefix() + table)));
Review Comment:
This can avoid a Text:
```suggestion
scanner.setRange(new Range(ProblemSection.getRowPrefix() +
table));
```
##########
server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReports.java:
##########
@@ -217,9 +218,9 @@ private Iterator<Entry<Key,Value>> getIter2() {
scanner.setTimeout(3, TimeUnit.SECONDS);
if (table == null) {
- scanner.setRange(new Range(new Text("~err_"), false, new
Text("~err`"), false));
+ scanner.setRange(ProblemSection.getRange());
Review Comment:
One difference is that the new version has the start key inclusive, and the
old range has it exclusive. I don't think this matters, since we don't expect
any rows to just include the prefix without a tableId after the underscore, so
this isn't an important change. I'm just noting it here as a very slight
difference in behavior, in case there's a surprise later, and this comment
serves to help debug something.
--
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]
