From d52d9e6e838fa5198f0bc40d1be6189350468260 Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Fri, 27 Mar 2026 17:46:34 +0000 Subject: [PATCH 1/2] Fix log message formats --- .../apache/accumulo/tserver/TabletServer.java | 19 +++++++++---------- .../tserver/log/TabletServerLogger.java | 6 +++--- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java index c91ac319c55..ff101d08ceb 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java @@ -222,8 +222,8 @@ protected TabletServer(ServerOpts opts, super(ServerId.Type.TABLET_SERVER, opts, serverContextFactory, args); context = super.getContext(); final AccumuloConfiguration aconf = getConfiguration(); - log.info("Version " + Constants.VERSION); - log.info("Instance " + getInstanceID()); + log.info("Version {}", Constants.VERSION); + log.info("Instance {}", getInstanceID()); this.sessionManager = new SessionManager(context); this.logSorter = new LogSorter(this); this.statsKeeper = new TabletStatsKeeper(); @@ -423,7 +423,7 @@ private HostAndPort getManagerAddress() { } return HostAndPort.fromString(managers.iterator().next().toHostPortString()); } catch (Exception e) { - log.warn("Failed to obtain manager host " + e); + log.warn("Failed to obtain manager host", e); } return null; @@ -438,7 +438,7 @@ private ManagerClientService.Client managerConnection(HostAndPort address) { // log.info("Listener API to manager has been opened"); return ThriftUtil.getClient(ThriftClientTypes.MANAGER, address, getContext()); } catch (Exception e) { - log.warn("Issue with managerConnection (" + address + ") " + e, e); + log.warn("Issue with managerConnection ({}) {}", address, e, e); } return null; } @@ -735,7 +735,7 @@ public TServerInstance getTabletSession() { try { return new TServerInstance(getAdvertiseAddress().toString(), lockSessionId); } catch (Exception ex) { - log.warn("Unable to read session from tablet server lock" + ex); + log.warn("Unable to read session from tablet server lock", ex); return null; } } @@ -1016,7 +1016,7 @@ private void markUnusedWALs() { try { TServerInstance session = this.getTabletSession(); for (DfsLogger candidate : eligible) { - log.info("Marking " + candidate.getPath() + " as unreferenced"); + log.info("Marking {} as unreferenced", candidate.getPath()); walMarker.walUnreferenced(session, candidate.getPath()); } synchronized (closedLogs) { @@ -1028,7 +1028,7 @@ private void markUnusedWALs() { } public void addNewLogMarker(DfsLogger copy) throws WalMarkerException { - log.info("Writing log marker for " + copy.getPath()); + log.info("Writing log marker for {}", copy.getPath()); walMarker.addNewWalMarker(getTabletSession(), copy.getPath()); } @@ -1040,7 +1040,7 @@ public void walogClosed(DfsLogger currentLog) throws WalMarkerException { closedLogs.add(currentLog); clSize = closedLogs.size(); } - log.info("Marking " + currentLog.getPath() + " as closed. Total closed logs " + clSize); + log.info("Marking {} as closed. Total closed logs {}", currentLog.getPath(), clSize); walMarker.closeWal(getTabletSession(), currentLog.getPath()); // whenever a new log is added to the set of closed logs, go through all of the tablets and @@ -1058,8 +1058,7 @@ public void walogClosed(DfsLogger currentLog) throws WalMarkerException { } } } else { - log.info( - "Marking " + currentLog.getPath() + " as unreferenced (skipping closed writes == 0)"); + log.info("Marking {} as unreferenced (skipping closed writes == 0)", currentLog.getPath()); walMarker.walUnreferenced(getTabletSession(), currentLog.getPath()); } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java index eccfff03ab1..c2a8eda6c31 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java @@ -293,7 +293,7 @@ private synchronized void startLogMaker() { try { tserver.addNewLogMarker(alog); } catch (Exception e) { - log.error("Failed to add new WAL marker for " + alog.getLogEntry(), e); + log.error("Failed to add new WAL marker for {}", alog.getLogEntry(), e); try { // Intentionally not deleting walog because it may have been advertised in ZK. See @@ -311,7 +311,7 @@ private synchronized void startLogMaker() { try { tserver.walogClosed(alog); } catch (WalMarkerException | RuntimeException e2) { - log.error("Failed to close WAL that failed to open: " + alog.getLogEntry(), e2); + log.error("Failed to close WAL that failed to open: {}", alog.getLogEntry(), e2); } try { @@ -348,7 +348,7 @@ private synchronized void close() throws IOException { } catch (DfsLogger.LogClosedException ex) { // ignore } catch (IOException | RuntimeException ex) { - log.error("Unable to cleanly close log " + currentLog.getLogEntry() + ": " + ex, ex); + log.error("Unable to cleanly close log {}: {}", currentLog.getLogEntry(), ex, ex); } finally { try { this.tserver.walogClosed(currentLog); From 934968c32d943080d316eacff8c19a5df826d2b5 Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Fri, 27 Mar 2026 18:00:07 +0000 Subject: [PATCH 2/2] Removes ServerContext from method signature Removes ServerContext from being passed around since a tserver object is already set within the logger. Also removed a reference to a VolumeManager that was not being used. --- .../apache/accumulo/tserver/TabletServer.java | 8 +++--- .../tserver/log/TabletServerLogger.java | 12 ++++----- .../accumulo/tserver/tablet/Tablet.java | 25 +++++++++---------- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java index ff101d08ceb..ca96f815f18 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java @@ -890,15 +890,15 @@ public boolean needsRecovery(TabletMetadata tabletMetadata) { } try { - return logger.needsRecovery(getContext(), tabletMetadata.getExtent(), logEntries); + return logger.needsRecovery(tabletMetadata.getExtent(), logEntries); } catch (IOException e) { throw new UncheckedIOException(e); } } - public void recover(VolumeManager fs, KeyExtent extent, Collection logEntries, - Set tabletFiles, MutationReceiver mutationReceiver) throws IOException { - logger.recover(getContext(), extent, logEntries, tabletFiles, mutationReceiver); + public void recover(KeyExtent extent, Collection logEntries, Set tabletFiles, + MutationReceiver mutationReceiver) throws IOException { + logger.recover(extent, logEntries, tabletFiles, mutationReceiver); } public int createLogId() { diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java index c2a8eda6c31..d5e345f9f8e 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java @@ -50,7 +50,6 @@ import org.apache.accumulo.core.util.Retry; import org.apache.accumulo.core.util.Retry.RetryFactory; import org.apache.accumulo.core.util.threads.Threads; -import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.log.WalStateManager.WalMarkerException; import org.apache.accumulo.tserver.TabletMutations; import org.apache.accumulo.tserver.TabletServer; @@ -523,26 +522,25 @@ private CacheProvider createCacheProvider(TabletServerResourceManager resourceMg LoggingBlockCache.wrap(CacheType.DATA, resourceMgr.getDataCache())); } - public boolean needsRecovery(ServerContext context, KeyExtent extent, Collection walogs) - throws IOException { + public boolean needsRecovery(KeyExtent extent, Collection walogs) throws IOException { try { var resourceMgr = tserver.getResourceManager(); var cacheProvider = createCacheProvider(resourceMgr); SortedLogRecovery recovery = - new SortedLogRecovery(context, resourceMgr.getFileLenCache(), cacheProvider); + new SortedLogRecovery(tserver.getContext(), resourceMgr.getFileLenCache(), cacheProvider); return recovery.needsRecovery(extent, resolve(walogs)); } catch (Exception e) { throw new IOException(e); } } - public void recover(ServerContext context, KeyExtent extent, Collection walogs, - Set tabletFiles, MutationReceiver mr) throws IOException { + public void recover(KeyExtent extent, Collection walogs, Set tabletFiles, + MutationReceiver mr) throws IOException { try { var resourceMgr = tserver.getResourceManager(); var cacheProvider = createCacheProvider(resourceMgr); SortedLogRecovery recovery = - new SortedLogRecovery(context, resourceMgr.getFileLenCache(), cacheProvider); + new SortedLogRecovery(tserver.getContext(), resourceMgr.getFileLenCache(), cacheProvider); recovery.recover(extent, resolve(walogs), tabletFiles, mr); } catch (Exception e) { throw new IOException(e); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java index fa38dceeb8e..84aac042c35 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java @@ -279,19 +279,18 @@ public Tablet(final TabletServer tabletServer, final KeyExtent extent, absPaths.add(ref.getNormalizedPathStr()); } - tabletServer.recover(this.getTabletServer().getVolumeManager(), extent, logEntries, - absPaths, m -> { - Collection muts = m.getUpdates(); - for (ColumnUpdate columnUpdate : muts) { - if (!columnUpdate.hasTimestamp()) { - // if it is not a user set timestamp, it must have been set - // by the system - maxTime.set(Math.max(maxTime.get(), columnUpdate.getTimestamp())); - } - } - getTabletMemory().mutate(commitSession, Collections.singletonList(m), 1); - entriesUsedOnTablet.incrementAndGet(); - }); + tabletServer.recover(extent, logEntries, absPaths, m -> { + Collection muts = m.getUpdates(); + for (ColumnUpdate columnUpdate : muts) { + if (!columnUpdate.hasTimestamp()) { + // if it is not a user set timestamp, it must have been set + // by the system + maxTime.set(Math.max(maxTime.get(), columnUpdate.getTimestamp())); + } + } + getTabletMemory().mutate(commitSession, Collections.singletonList(m), 1); + entriesUsedOnTablet.incrementAndGet(); + }); if (maxTime.get() != Long.MIN_VALUE) { tabletTime.updateTimeIfGreater(maxTime.get());