ctubbsii commented on code in PR #5909:
URL: https://github.com/apache/accumulo/pull/5909#discussion_r2370239702
##########
test/src/main/java/org/apache/accumulo/test/functional/OnDemandTabletUnloadingFlakyAmpleIT.java:
##########
@@ -65,7 +67,9 @@ public void testOndemandFlakyUnload() throws Exception {
String tableName = super.getUniqueNames(1)[0];
- c.tableOperations().create(tableName);
+ NewTableConfiguration ntc = new NewTableConfiguration();
+
ntc.setProperties(Map.of("table.custom.ondemand.unloader.inactivity.threshold.seconds",
"3"));
Review Comment:
There is a constant in DefaultOnDemandTabletUnloader. We don't have to use
the string literal everywhere. We can use the constant.
Also... I didn't see this before, but we should renamed
"DefaultOnDemandTabletUnloader". As I've mentioned in other similar situations,
user-configurable classes that have the name "Default" in them because we
currently have them as the default implementation, is unhelpful in two ways:
1. The name isn't future-proof. We've actually had the situation before
where the one named "Default" wasn't the currently configured default, and that
is very confusing. Even if it's currently configured as the default, that
doesn't necessarily mean it will be in the future.
2. The name doesn't tell the user anything about what it does. The
implementation name should generally reflect the strategy it employs. For
example, "The default strategy is the RoundRobinStrategy" vs. "The default
strategy is the DefaultStrategy". When a simple implementation doesn't lend
itself to a clear and obvious name, the name "Simple" can be a good alternative
to "Default".
The rename could be done in a different PR, but it's a small enough change
that it could also be included here.
--
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]
