You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/06/01 15:17:07 UTC

[GitHub] [pulsar-client-go] jiangbo9510 opened a new pull request #530: [Issue 527] the pull request Title

jiangbo9510 opened a new pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530


   
   Fix Issue: [#<527>](https://github.com/apache/pulsar-client-go/issues/527)
   
   ### Motivation
   
   I add a function to compare and poll block queue in atomic. To fix poll a wrong item in `failTimeoutMessages()`
   
   ### Modifications
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   ### Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API: (no)
     - The schema: (no)
     - The default values of configurations: (no)
     - The wire protocol: (no)
   
   ### Documentation
   
     - Does this pull request introduce a new feature? (no)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 commented on a change in pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 commented on a change in pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#discussion_r643263650



##########
File path: pulsar/producer_partition.go
##########
@@ -490,21 +490,36 @@ func (p *partitionProducer) failTimeoutMessages() {
 
 		// iterate at most viewSize items
 		for i := 0; i < viewSize; i++ {
-			item := p.pendingQueue.Poll()
+			tickerNeedWaiting := time.Duration(0)
+			item := p.pendingQueue.CompareAndPoll(

Review comment:
       > This should also be used to refactor the `pendingQueue.Peek()` at line 464
   
   @merlimat  There is not necessarily.  pendingQueue.Peek()` at line 464 is to check the first item of queue whether timeout. It not modify the queue.
   
   
   
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] wolfstudy commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-872851917


   cc / @cckellogg @merlimat  PTAL


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-854328236


   @merlimat  please review my code and merge this pull request to master branch.
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] wolfstudy merged pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
wolfstudy merged pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] wolfstudy commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-878121995


   @jiangbo9510 https://github.com/apache/pulsar-client-go/issues/565


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-853794939






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] merlimat commented on a change in pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#discussion_r643230390



##########
File path: pulsar/producer_partition.go
##########
@@ -490,21 +490,36 @@ func (p *partitionProducer) failTimeoutMessages() {
 
 		// iterate at most viewSize items
 		for i := 0; i < viewSize; i++ {
-			item := p.pendingQueue.Poll()
+			tickerNeedWaiting := time.Duration(0)
+			item := p.pendingQueue.CompareAndPoll(

Review comment:
       This should also be used to refactor the `pendingQueue.Peek()` at line 464




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-879915467


   > @jiangbo9510 #565
   
   @wolfstudy  It's anothor problem. If it occured,All send goroutins will blocked by the dead lock.
   
   In my case, Sometimes some send goroutins blocked. and others goroutins can send message success.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 edited a comment on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 edited a comment on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-874686308


   > @jiangbo9510 Please merger master code for failed action CI
   
   @wolfstudy 
   What does this mean?
   Should I merge master to my branch?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-853794939


   @wolfstudy  When this pull request merge to the master branch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 edited a comment on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 edited a comment on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-879915467


   > @jiangbo9510 #565
   
   @wolfstudy  It's anothor problem. If it occured,All send goroutins will blocked by the dead lock.
   
   In my case, Sometimes some send goroutins blocked. and others goroutins can send message successful.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] ideadsnow commented on a change in pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
ideadsnow commented on a change in pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#discussion_r643272640



##########
File path: pulsar/producer_partition.go
##########
@@ -490,21 +490,36 @@ func (p *partitionProducer) failTimeoutMessages() {
 
 		// iterate at most viewSize items
 		for i := 0; i < viewSize; i++ {
-			item := p.pendingQueue.Poll()
+			tickerNeedWaiting := time.Duration(0)
+			item := p.pendingQueue.CompareAndPoll(

Review comment:
       the bug reason is peek head item and determine if it timeout, and then get a snapshot of pendingQueue, range snapshot and poll all the items in snapshot even if it is not all timeout.
   the pendingQueue.Peek() at line 464 just peek(not poll) the head item in queue and check(logically correct), there are no actual operation to pendingQueue, so it looks like no need to refactor




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] jiangbo9510 commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
jiangbo9510 commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-874686308


   > @jiangbo9510 Please merger master code for failed action CI
   
   What does this mean?
   Should I merge master to my branch?
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] wolfstudy commented on pull request #530: [Issue 527] fix send goroutine blocked

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #530:
URL: https://github.com/apache/pulsar-client-go/pull/530#issuecomment-872856690


   @jiangbo9510 Please merger master code for failed action CI


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org