You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2025/08/13 13:00:22 UTC
[PR] Fixed issue with exception in server run method not being logged [accumulo]
dlmarion opened a new pull request, #5796:
URL: https://github.com/apache/accumulo/pull/5796
The use of try-with-resources in the server process' main method causes the server's close method to always be called. When an exception occurs in the runServer method it may not be logged because the invocation of the close method consumes enough time that the lock watcher is fired which ends up halting the JVM.
Closes #5794
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue with exception in server run method not being logged [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #5796:
URL: https://github.com/apache/accumulo/pull/5796#issuecomment-3191423789
> The quick fix is to log the message first and then try to clean up in the close method... which will trigger the halt.
Yeah, that's fair. I can modify this PR to do just that.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue with exception in server run method not being logged [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on PR #5796:
URL: https://github.com/apache/accumulo/pull/5796#issuecomment-3184544908
If an exception is thrown in the `.runServer()` methods, then `.close()` is never called. Does that cause any issues?
> The above was never logged, instead the only thing that I saw in the log was the fact that the server's lock was gone and that the JVM was being halted.
Is this due to the ZK lock being lost by closing the client context? I'm curious that one logger is able to print a message showing the lock is gone, but the exception message wasn't flushed.
I thought try-with-resources will append any exception in the `close()` method with the one from the original resource.
I wonder if it "holding onto" the original exception message is why it's not ending up in the error log.
In general, if we don't care about any cleanup actions being called in the `.close()` then this change seems fine, but I'm not 100% sure it solves the problem.
Could the exception be logged in `AbstractServer.runServer()` before throwing the exception up to the try-with-resources block in the various Main methods?
```
Throwable thrown = err.get();
if (thrown != null) {
log.error(thrown.getMessage());
if (thrown instanceof Error) {
throw (Error) thrown;
}
if (thrown instanceof Exception) {
throw (Exception) thrown;
}
throw new RuntimeException("Weird throwable type thrown", thrown);
}
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue with exception in server run method not being logged [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #5796:
URL: https://github.com/apache/accumulo/pull/5796
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue with exception in server run method not being logged [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #5796:
URL: https://github.com/apache/accumulo/pull/5796#issuecomment-3184619057
> If an exception is thrown in the `.runServer()` methods, then `.close()` is never called. Does that cause any issues?
>
I don't think so. ServerContext.close does call close on the shared conditional writers for the user and metadata tables, but I think if an exception is thrown from server's run method, then we want to exit as quickly as possible.
> > The above was never logged, instead the only thing that I saw in the log was the fact that the server's lock was gone and that the JVM was being halted.
>
> Is this due to the ZK lock being lost by closing the client context? I'm curious that one logger is able to print a message showing the lock is gone, but the exception message wasn't flushed.
It's due to close being called and the JVM halting due to the lock watcher firing before the code in `Main.die` executes.
>
> I thought try-with-resources will append any exception in the `close()` method with the one from the original resource. I wonder if it "holding onto" the original exception message is why it's not ending up in the error log.
>
> In general, if we don't care about any cleanup actions being called in the `.close()` then this change seems fine, but I'm not 100% sure it solves the problem and without code comments, I could see this change getting accidentally refactored back into try-with resource blocks.
>
> Could the exception be logged in `AbstractServer.runServer()` before throwing the exception up to the try-with-resources block in the various Main methods?
>
> ```
> Throwable thrown = err.get();
> if (thrown != null) {
> log.error(thrown.getMessage());
> if (thrown instanceof Error) {
> throw (Error) thrown;
> }
> if (thrown instanceof Exception) {
> throw (Exception) thrown;
> }
> throw new RuntimeException("Weird throwable type thrown", thrown);
> }
> ```
Yes, we could do that. I did do that and I saw the logging show up. The place where the logging is done currently is in `Main.die`, called from `Main.execKeyword`.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue with exception in server run method not being logged [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5796:
URL: https://github.com/apache/accumulo/pull/5796#discussion_r2283987855
##########
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java:
##########
@@ -1064,7 +1064,13 @@ public void run() {
public static void main(String[] args) throws Exception {
try (Compactor compactor = new Compactor(new ConfigOpts(), args)) {
- compactor.runServer();
+ try {
+ compactor.runServer();
+ } catch (Exception e) {
+ System.err.println("Compactor died, exception thrown from runServer.");
+ e.printStackTrace();
+ LOG.error("Compactor died, exception thrown from runServer.", e);
+ }
Review Comment:
This is fine but causes it to exit with a normal exit code, 0. Normally an exception being thrown through the main class would cause a non-zero exit code.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue with exception in server run method not being logged [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #5796:
URL: https://github.com/apache/accumulo/pull/5796#issuecomment-3201080594
> Bad: In the case of a clean exit, for a graceful shutdown, I think the background thread could still halt the process and produce an exception and non-zero exit code, but as I've mentioned, it's okay if this PR doesn't try to address that specific case.
The lock watcher may halt the process, but would do so with a `0` exit code. All of the server processes `run` methods end in this same general pattern:
https://github.com/apache/accumulo/blob/2570c5069433d1d72cb029541c8c34ba8f7e14fe/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java#L712-L720
The last thing that happens is unlocking the lock, which may end up causing the ServiceLockWatcher to fire before the JVM ends. However, it uses a `0` exit code in this case because the shutdownComplete object is set to true.
https://github.com/apache/accumulo/blob/2570c5069433d1d72cb029541c8c34ba8f7e14fe/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java#L165-L170
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixed issue with exception in server run method not being logged [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #5796:
URL: https://github.com/apache/accumulo/pull/5796#issuecomment-3183885989
I found this issue because I started 4.0 software on a 2.1.4 instance without running the `upgrade --start` command. I expected to see the following in the error log:
```
Thread 'manager' died.
java.lang.IllegalStateException: Upgrade not started, /upgrade_progress node does not exist. Did you run 'accumulo upgrade --start'?
at org.apache.accumulo.server.util.upgrade.UpgradeProgressTracker.continueUpgrade(UpgradeProgressTracker.java:81)
at org.apache.accumulo.manager.upgrade.UpgradeCoordinator.continueUpgrade(UpgradeCoordinator.java:159)
at org.apache.accumulo.manager.Manager.setManagerState(Manager.java:365)
at org.apache.accumulo.manager.Manager.run(Manager.java:953)
at org.apache.accumulo.core.trace.TraceWrappedRunnable.run(TraceWrappedRunnable.java:52)
at java.base/java.lang.Thread.run(Thread.java:840)
```
The above was never logged, instead the only thing that I saw in the log was the fact that the server's lock was gone and that the JVM was being halted.
AbstractServer.close was an empty method until #5726, which was included as part of the 2.1.4 release.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org