- 
                Notifications
    
You must be signed in to change notification settings  - Fork 963
 
[fix] Bookie Info lost by notification race condition. #20642 #4646
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
base: master
Are you sure you want to change the base?
Conversation
| 
           Can you provide some information about the process of locating the problem you encountered, or is this just a simple logic synchronization with Pulsar?  | 
    
| 
           @StevenLuMT  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a race condition in BookKeeper's ZKRegistrationClient where bookie information could be lost due to notification timing issues. The fix separates the cache for writable and read-only bookies and ensures sequential processing of ZooKeeper events to prevent race conditions.
- Splits the single 
bookieServiceInfoCacheinto separate caches for writable and read-only bookies - Introduces sequential processing of ZooKeeper notifications using a 
Sequencerutility class - Adds comprehensive test coverage to verify the fix handles network delays during bookie transitions
 
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description | 
|---|---|
ZKRegistrationClient.java | 
Core fix implementing separate caches and sequential event processing | 
FutureUtils.java | 
Adds Sequencer utility class for sequential task execution | 
ZKRegistrationClientTest.java | 
New test to verify race condition fix with network delay simulation | 
FaultInjectableZKRegistrationManager.java | 
Test utility for fault injection during registration operations | 
        
          
                bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java
          
            Show resolved
            Hide resolved
        
              
          
                bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationClient.java
          
            Show resolved
            Hide resolved
        
      | 
           rerun failure checks  | 
    
    
      
        1 similar comment
      
    
  
    | 
           rerun failure checks  | 
    
| 
           @gaozhangmin Is it different from this fix #4481  | 
    
          
 it's different case  | 
    
27b9b85    to
    634bf4f      
    Compare
  
    634bf4f    to
    40e60d5      
    Compare
  
    | 
           rerun failure checks  | 
    
According to pulsar's fix: apache/pulsar#20642
race conditions problem is also exists in bk's ZKRegistrationClient.java