Examine multi-thread correctness in ZooPropstore.get()
- 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
Assessment
This issue has not been assessed yet.