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.

Sign up for GitHub

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

Jump to bottom

When the Kafka broker is throttled, the producer is easily blocked #2823

Closed
chengsha opened this issue Mar 3, 2024 · 5 comments · Fixed by #2826
Closed

When the Kafka broker is throttled, the producer is easily blocked #2823

chengsha opened this issue Mar 3, 2024 · 5 comments · Fixed by #2826
Labels
needs-investigation Issues that require followup from maintainers

Comments

@chengsha
Copy link
Contributor

chengsha commented Mar 3, 2024

Description

When the Kafka broker is throttled, the producer is easily blocked.
I find data race in setThrottle and waitIfThrottled.

Versions
Sarama Kafka Go
v1.43.0 2.4.1 1.21.5
Configuration


Logs


WARNING: DATA RACE

Write at 0x00c000598f80 by goroutine 93:

github.com/IBM/sarama.(*Broker).setThrottle()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:1706 +0xe4

github.com/IBM/sarama.(*Broker).handleThrottledResponse()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:1695 +0x208

github.com/IBM/sarama.(*Broker).AsyncProduce.func1()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:467 +0x117

github.com/IBM/sarama.(*responsePromise).handle()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:140 +0x148

github.com/IBM/sarama.(*Broker).responseReceiver()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:1210 +0x584

github.com/IBM/sarama.(*Broker).responseReceiver-fm()

:1 +0x33

github.com/IBM/sarama.withRecover()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/utils.go:43 +0x41

github.com/IBM/sarama.(*Broker).Open.func1.5()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:258 +0x33


Previous read at 0x00c000598f80 by goroutine 77:

github.com/IBM/sarama.(*Broker).waitIfThrottled()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:1710 +0x44

github.com/IBM/sarama.(*Broker).sendInternal()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:1010 +0x307

github.com/IBM/sarama.(*Broker).sendWithPromise()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:994 +0xdd

github.com/IBM/sarama.(*Broker).AsyncProduce()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:473 +0x3a5

github.com/IBM/sarama.(*asyncProducer).newBrokerProducer.func1()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/async_producer.go:829 +0x264

github.com/IBM/sarama.withRecover()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/utils.go:43 +0x41

github.com/IBM/sarama.(*asyncProducer).newBrokerProducer.func4()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/async_producer.go:794 +0x33


Goroutine 93 (running) created at:

github.com/IBM/sarama.(*Broker).Open.func1()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:258 +0x1dea

github.com/IBM/sarama.withRecover()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/utils.go:43 +0x41

github.com/IBM/sarama.(*Broker).Open.func2()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/broker.go:184 +0x33


Goroutine 77 (running) created at:

github.com/IBM/sarama.(*asyncProducer).newBrokerProducer()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/async_producer.go:794 +0x690

github.com/IBM/sarama.(*asyncProducer).getBrokerProducer()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/async_producer.go:1343 +0x133

github.com/IBM/sarama.(*partitionProducer).dispatch()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/async_producer.go:634 +0x1a9

github.com/IBM/sarama.(*partitionProducer).dispatch-fm()

:1 +0x33

github.com/IBM/sarama.withRecover()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/utils.go:43 +0x41

github.com/IBM/sarama.(*asyncProducer).newPartitionProducer.func1()

/data/gopath/pkg/mod/github.com/!i!b!m/sarama@v1.43.0/async_producer.go:600 +0x33


logs: CLICK ME











Additional Context


i can resove this bug by add lock

image

      		
    

      
  





        


            

              
            

        

      


      
    








      

    



      

  
          

  

    
  

  


    

        @dnwe
dnwe




        added
  the 

  needs-investigation

  Issues that require followup from maintainers
 label


    Mar 4, 2024

  










      

  
      



  

  @dnwe





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      

  
    Collaborator


      

  


  

    

      

      
          dnwe
  

      

      

      commented


        Mar 4, 2024


      
    


  





      


        


  
    

      
          
@chengsha thanks for reporting! Are you happy to submit a PR with your proposed change?

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
          


  
    
  
  

    
  chengsha 

      pushed a commit
        to chengsha/sarama
      that referenced
      this issue

    
      Mar 5, 2024
    
    

      


  

    

      


  

  





      

        
          The producer can be easily blocked due to a race condition in Broker.…
        

          
            
          

      


      

          

    
    
    

  

      


      

      


      
      

        
          45e18cf
        
      

    

  

    

      
…throttleTimer, which may result in a panic. Consider changing the type of Broker.throttleTimer to atomic.Value

Fixes IBM#2823

Signed-off-by: chengsha  <shacheng@tencent.com>

    




    

  




  

  

    
  


  

      

    @chengsha
chengsha



    mentioned this issue
    
      Mar 5, 2024
    





  


  

      
   Merged

  







  









      

  
      



  

  @chengsha





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      

  
    Contributor


      

  Author


  


  

    

      

      
          chengsha
  

      

      

      commented


        Mar 5, 2024


      
    


  





      


        


  
    

      
          


@chengsha thanks for reporting! Are you happy to submit a PR with your proposed change?




ok, I use atomic.Value  instead of lock

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
          


  
    
  
  

    
  chengsha 

      pushed a commit
        to chengsha/sarama
      that referenced
      this issue

    
      Mar 6, 2024
    
    

      


  

    

      


  

  





      

        
          The producer can be easily blocked due to a race condition in Broker.…
        

          
            
          

      


      

          

    
    
    

  

      


      

      


      
      

        
          c6d9d39
        
      

    

  

    

      
…throttleTimer, which may result in a panic. add throttleTimerLock to protect

Fixes IBM#2823

Signed-off-by: chengsha  <shacheng@tencent.com>

    




    

  




  


  
    
  
  

    
  chengsha 

      pushed a commit
        to chengsha/sarama
      that referenced
      this issue

    
      Mar 6, 2024
    
    

      


  

    

      


  

  





      

        
          The producer can be easily blocked due to a race condition in Broker.…
        

          
            
          

      


      

          

    
    
    

  

      


      

      


      
      

        
          cc3fa66
        
      

    

  

    

      
…throttleTimer, which may result in a panic. add throttleTimerLock to protect

Fixes IBM#2823

Signed-off-by: chengsha  <shacheng@tencent.com>

    




    

  




  


  
    
  
  

    
  chengsha 

      pushed a commit
        to chengsha/sarama
      that referenced
      this issue

    
      Mar 7, 2024
    
    

      


  

    

      


  

  





      

        
          The producer can be easily blocked due to a race condition in Broker.…
        

          
            
          

      


      

          

    
    
    

  

      


      

      


      
      

        
          7d6b70b
        
      

    

  

    

      
…throttleTimer, which may result in a panic. add throttleTimerLock to protect

Fixes IBM#2823

Signed-off-by: shacheng <shacheng@tencent.com>

    




    

  




  

  

    
  

  


    

        @dnwe
dnwe




          closed this as completed
  in
    
      #2826


    Mar 8, 2024

  




  


  


  
    
  
  

    
  dnwe 

      pushed a commit
      that referenced
      this issue

    
      Mar 8, 2024
    
    

      


  

    

      


  

      
        @chengsha
  





      

        
          fix: add locking around broker throttle timer to prevent race conditi…
        

          
            
          

      


      

          

    
    
    

  

    


      


      

          

    
    

  

  

    
  



      


      
      

        
          a8b3b3d
        
      

    

  

    

      
…on (#2826)

Fixes #2823

Signed-off-by: shacheng <shacheng@tencent.com>
Co-authored-by: shacheng <shacheng@tencent.com>

    




    

  









      

  
      



  

  @korrawit





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      



      

  


  

    

  





      


        


  
    

      
          
@dnwe Would it be feasible to consider releasing this as a hotfix? We've encountered these issues repeatedly in our production environment, and addressing them promptly is imperative. Thank you for your attention to this matter.

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
      



  

  @dnwe





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      

  
    Collaborator


      

  


  

    

      

      
          dnwe
  

      

      

      commented


        Mar 27, 2024


      
    


  





      


        


  
    

      
          
@korrawit ah yes, I thought I already had done — I'll make a release shortly

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    












      

  
      



  

  @dnwe





  


    

      

  

    

        
    
    

  


      
          
    
            Copy link

      
    

  


  

      

      

  
    Collaborator


      

  


  

    

      

      
          dnwe
  

      

      

      commented


        Mar 27, 2024


      
    


  





      


        


  
    

      
          
Released in v1.43.1

      		
    

  





        


            

              
            

        

      


      

            
  

    
  
  

  

  



    
















  
  

    

      

  





  




  
  

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



  






  
                  




    

  


      
  

    Assignees
  



        

    No one assigned







      


  


  

    Labels
  



    

      

    needs-investigation

  Issues that require followup from maintainers








      

  

    

        

    Projects
  


        




    None yet




  



      

  

    
  

    Milestone
  


      No milestone





    
      
          


  

    
      

        
    

    Development
  




            
Successfully merging a pull request may close this issue.





    


  
          
      fix: add locking around broker throttle timer to prevent race condition

    
      chengsha/sarama
    





    
  




        
      

    

  
      

  

    

      3 participants
    

    

        
          @korrawit 
        
          @chengsha 
        
          @dnwe