apache / apache/accumulo

Examine multi-thread correctness in ZooPropstore.get()

Open
#2,689 8 comments 0 reactions 2 assignees Claimed by @keith-turner View on GitHub
bug
Dominant language
Java
Stars
1.2k
Forks
487
Avg merge
4d 5h
Merged PRs (30d)
13

Description

From PR Comment ZooPropStore.get() method - this can be reviewed as a follow-on to the single node prop store changes.

@keith-turner said:

Tracing through the code, I think the following sequence of events could happen. Does this seem correct? Want to make sure I am following the code correctly.

Thread 1 calls checkZkConnection() and does not block because the conneciton is currently ok
Thread 2 sees connection loss event in watcher and clears the ready and queues up an event to clear the cache
Thread 3 clears the cache (running in an executor on behalf of event queued by thread 2)
Thread 1 executes cache.get()... because the cache was cleared, a loader is executed in a thread pool
Thread 4 executes the code to load data from ZK... the connection is still lost, so what happens here? This thread is executing on behalf of the cache.get() initiated by thread 1.
I am not sure exactly what happens in step 5 above, does the load from ZK retrty when it sees a connection lost exception? If it does then the checkZkConnection() is redundant. If it does not then the checkZkConnection() does not always prevent problems in the case of connection loss, and maybe it should retry inorder to eliminate the race condition.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.