keith-turner commented on code in PR #4435:
URL: https://github.com/apache/accumulo/pull/4435#discussion_r1596044021
##########
server/base/src/main/java/org/apache/accumulo/server/compaction/CompactionJobGenerator.java:
##########
@@ -87,42 +88,61 @@ public CompactionJobGenerator(PluginEnvironment env,
unknownCompactionServiceErrorCache =
Caches.getInstance().createNewBuilder(CacheName.COMPACTION_SERVICE_UNKNOWN,
false)
.expireAfterWrite(5, TimeUnit.MINUTES).build();
+
+ generateJobsErrorCache =
+
Caches.getInstance().createNewBuilder(CacheName.COMPACTION_SERVICE_UNKNOWN,
false)
+ .expireAfterWrite(5, TimeUnit.MINUTES).build();
+
}
public Collection<CompactionJob> generateJobs(TabletMetadata tablet,
Set<CompactionKind> kinds) {
+ try {
+ Collection<CompactionJob> systemJobs = Set.of();
- // ELASTICITY_TODO do not want user configured plugins to cause exceptions
that prevents tablets
- // from being
- // assigned. So probably want to catch exceptions and log, but not too
spammily OR some how
- // report something
- // back to the manager so it can log.
-
- Collection<CompactionJob> systemJobs = Set.of();
+ if (kinds.contains(CompactionKind.SYSTEM)) {
+ CompactionServiceId serviceId = dispatch(CompactionKind.SYSTEM,
tablet, Map.of());
+ systemJobs = planCompactions(serviceId, CompactionKind.SYSTEM, tablet,
Map.of());
+ }
- if (kinds.contains(CompactionKind.SYSTEM)) {
- CompactionServiceId serviceId = dispatch(CompactionKind.SYSTEM, tablet,
Map.of());
- systemJobs = planCompactions(serviceId, CompactionKind.SYSTEM, tablet,
Map.of());
- }
+ Collection<CompactionJob> userJobs = Set.of();
- Collection<CompactionJob> userJobs = Set.of();
+ if (kinds.contains(CompactionKind.USER) && tablet.getSelectedFiles() !=
null) {
+ var hints =
allExecutionHints.get(tablet.getSelectedFiles().getFateId());
+ if (hints != null) {
+ CompactionServiceId serviceId = dispatch(CompactionKind.USER,
tablet, hints);
+ userJobs = planCompactions(serviceId, CompactionKind.USER, tablet,
hints);
+ }
+ }
- if (kinds.contains(CompactionKind.USER) && tablet.getSelectedFiles() !=
null) {
- var hints = allExecutionHints.get(tablet.getSelectedFiles().getFateId());
- if (hints != null) {
- CompactionServiceId serviceId = dispatch(CompactionKind.USER, tablet,
hints);
- userJobs = planCompactions(serviceId, CompactionKind.USER, tablet,
hints);
+ if (userJobs.isEmpty()) {
+ return systemJobs;
+ } else if (systemJobs.isEmpty()) {
+ return userJobs;
+ } else {
+ var all = new ArrayList<CompactionJob>(systemJobs.size() +
userJobs.size());
+ all.addAll(systemJobs);
+ all.addAll(userJobs);
+ return all;
+ }
+ } catch (Exception e) {
+ // The same code that plans compactions also assigns tablets. The intent
of this catch is
+ // mainly to defend against user plugins called here that throw an
exception from negatively
+ // impacting tablet assignment.
+ String cacheKey = tablet.getTableId() + " " + e.getClass().getName();
+ // This check defends against every tablet in a table having the same
problem and therefore
+ // generating an enormous amount of spam for the logs.
+ var last = generateJobsErrorCache.getIfPresent(cacheKey);
Review Comment:
Looking at the docs for get it will return the computed value if absent.
Can get the prev value w/ putIfAbsent. Made that change in d246ee8
--
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]
