keith-turner commented on code in PR #2804:
URL: https://github.com/apache/accumulo/pull/2804#discussion_r922301056
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java:
##########
@@ -1105,23 +1105,23 @@ public Optional<Files> getFiles(CompactionServiceId
service, CompactionKind kind
}
class CompactionCheck {
- private final Supplier<Boolean> memoizedCheck;
+ private final Supplier<Boolean> expensiveCheck;
+ private final Supplier<Boolean> inexpensiveCheck;
public CompactionCheck(CompactionServiceId service, CompactionKind kind,
Long compactionId) {
- this.memoizedCheck = Suppliers.memoizeWithExpiration(() -> {
- if (closed)
+ this.expensiveCheck = Suppliers.memoizeWithExpiration(() -> {
+ return service.equals(getConfiguredService(kind));
+ }, 3, TimeUnit.SECONDS);
+ this.inexpensiveCheck = Suppliers.memoizeWithExpiration(() -> {
+ if (closed
+ || (kind == CompactionKind.USER &&
lastSeenCompactionCancelId.get() >= compactionId))
return false;
- if (!service.equals(getConfiguredService(kind)))
- return false;
- if (kind == CompactionKind.USER && lastSeenCompactionCancelId.get() >=
compactionId)
- return false;
-
return true;
- }, 100, TimeUnit.MILLISECONDS);
+ }, 50, TimeUnit.MILLISECONDS);
Review Comment:
Was there a specific reason to adjust this time from 100 to 50? The initial
100ms time was arbitrarily chosen, so not in any way opposed to changing it.
Just curious if you saw something that prompted this.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java:
##########
@@ -1105,23 +1105,23 @@ public Optional<Files> getFiles(CompactionServiceId
service, CompactionKind kind
}
class CompactionCheck {
- private final Supplier<Boolean> memoizedCheck;
+ private final Supplier<Boolean> expensiveCheck;
+ private final Supplier<Boolean> inexpensiveCheck;
public CompactionCheck(CompactionServiceId service, CompactionKind kind,
Long compactionId) {
- this.memoizedCheck = Suppliers.memoizeWithExpiration(() -> {
- if (closed)
+ this.expensiveCheck = Suppliers.memoizeWithExpiration(() -> {
+ return service.equals(getConfiguredService(kind));
Review Comment:
How was it determined this check was expensive?
--
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]
