Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace nonpublic Accumulo Threads with new Datawave equivalent #2520

Closed
wants to merge 0 commits into from

Conversation

SethSmucker
Copy link
Collaborator

org.apache.accumulo.core.util.threads.Threads is not part of Accumulo's public API. The variable UEH is the only part of the class used in Datawave.

Created datawave.util.threads.DatawaveUncaughtExceptionHandler and datawave.util.threads.Threads which are similar to the Accumulo classes.

Replace all references of Accumulo's Threads and AccumuloUncaughtExceptionHandler with the Datawave classes.

part of #2443

@SethSmucker
Copy link
Collaborator Author

This is the bare minimum to move to Datawave. If you'd like me to include the same functionality as the Accumulo version I can do that.

alerman
alerman previously approved these changes Aug 22, 2024
avgAGB
avgAGB previously approved these changes Aug 23, 2024
jschmidt10
jschmidt10 previously approved these changes Aug 26, 2024
@SethSmucker SethSmucker dismissed stale reviews from avgAGB and jschmidt10 via 001e89f August 29, 2024 18:20
// If e == OutOfMemoryError, then it's probably that another Error might be
// thrown when trying to print to System.err.
} finally {
Runtime.getRuntime().halt(-1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I really do not like halting. I realize the accumulo one did this, but in the context of a BulkInputFormat, we certainly do not want to halt the vm which would most likely be a yarn container.
Instead, lets save off the exception, and then add finaly blocks above to check if exceptions were thrown so that we can pass them back up.

Copy link
Collaborator

@ivakegg ivakegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment

*/
@Override
public void uncaughtException(Thread t, Throwable e) {
Throwable savedException = null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SOrry, I was not clear. This savedException should be a member of this class. In the BulkInputFormat we should check whether that member has an exception and subsequently pass the exception up. I don't think throwing an exception in this method would actually be passed up as it would be in a separate thread.

@ivakegg
Copy link
Collaborator

ivakegg commented Sep 12, 2024

Also why all of the changes to the docker files in this PR ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants