[
https://issues.apache.org/jira/browse/ACCUMULO-1292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14287867#comment-14287867
]
Josh Elser commented on ACCUMULO-1292:
--------------------------------------
Thanks for the details last night, Dave. I think I know where my confusion was
coming from now. You're trying to avoid blocking all threads from executing
knowing that the classloader needs to be refreshed at the cost of continuing to
run with "stale" artifacts? That's why you wanted to use the async thread.
Assuming I'm on the same page now, that makes a lot more sense. What would
happen with the VFS classloader if a filesystem change happened (Jar was
replaced), refresh on the classloader is started but takes a long time, class
load is requested for a class that was from the replaced jar? Is that safe --
it would continue to load the old version of the class? Would requests before
the classloader is updated fail?
{code}
+ private final ThreadPoolExecutor executor =
+ new ThreadPoolExecutor(1, 1, 1, SECONDS, new
ArrayBlockingQueue<Runnable>(2));
{code}
Wouldn't {{Executors.newSingleThreadExecutor()}} be more concise? Is your
keepAliveTime actually doing to do anything with a coreSize of 1?
{code}
+ private void scheduleRefresh() {
+ try {
+ executor.execute(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ recreateClassloaderNow();
+ } catch (FileSystemException e) {
+ log.error("Error refreshing classloader", e);
+ clRef.set(null);
+ }
+ }
+ });
+ } catch (RejectedExecutionException e) {
+ // don't care, queue is full
+ }
+ }
{code}
Make the Runnable once. You don't need to make a new instance of it every time
you call this method. Also, debug or trace level logging in the catch?
{code}
public void close() {
+ executor.shutdown();
monitor.stop();
}
{code}
This will initiate the shutdown, but doesn't wait for it to finish. If the
tserver is trying to stop but is stuck trying to reload a class, we don't want
it to block the tserver from exiting (especially if the remote IO eats the
interrupted exception). Need to make sure that the async refreshing thread is a
daemon or provide a way to stop the thread.
I see you made some modifications to AccumuloVFSClassLoaderTest as well, but it
doesn't look like those are actually testing anything related to these changes.
Have you put any thought into how you can verify the correctness of these
changes?
> Tablet constructor can hang on vfs classloader, preventing tablets from
> loading
> -------------------------------------------------------------------------------
>
> Key: ACCUMULO-1292
> URL: https://issues.apache.org/jira/browse/ACCUMULO-1292
> Project: Accumulo
> Issue Type: Bug
> Components: tserver
> Affects Versions: 1.5.0, 1.6.0, 1.6.1
> Reporter: John Vines
> Assignee: Eric Newton
> Fix For: 1.7.0, 1.6.3
>
> Attachments: ACCUMULO-1292-atomic-update.patch,
> ACCUMULO-1292-using-locks.patch, ACCUMULO-1292.patch
>
>
> Taken from TODO from r1424106 regarding ACCUMULO-867. This is something that
> we should at least look into more before 1.5 is released.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
