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 2022/10/30 06:07:28 UTC
[GitHub] [pulsar-client-go] billowqiu opened a new pull request, #878: [Issue 877]Fix send stuck
billowqiu opened a new pull request, #878:
URL: https://github.com/apache/pulsar-client-go/pull/878
Fixes #877
### Motivation
Use select to avoid channel stuck.
### Modifications
- like consumer.Receive func, use for...select idiom.
--
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] RobertIndie closed pull request #878: [Issue 877]Fix send stuck
Posted by "RobertIndie (via GitHub)" <gi...@apache.org>.
RobertIndie closed pull request #878: [Issue 877]Fix send stuck
URL: https://github.com/apache/pulsar-client-go/pull/878
--
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] RobertIndie commented on a diff in pull request #878: [Issue 877]Fix send stuck
Posted by "RobertIndie (via GitHub)" <gi...@apache.org>.
RobertIndie commented on code in PR #878:
URL: https://github.com/apache/pulsar-client-go/pull/878#discussion_r1190912395
##########
pulsar/producer_partition.go:
##########
@@ -1035,9 +1035,13 @@ func (p *partitionProducer) Send(ctx context.Context, msg *ProducerMessage) (Mes
}, true)
// wait for send request to finish
- <-doneCh
-
- return msgID, err
+ select {
+ case <-ctx.Done():
+ isDone.Store(true)
+ return msgID, ctx.Err()
+ case <-doneCh:
+ return msgID, err
Review Comment:
We shouldn't return msgID and err here. They are not assigned here.
--
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] billowqiu commented on a diff in pull request #878: [Issue 877]Fix send stuck
Posted by GitBox <gi...@apache.org>.
billowqiu commented on code in PR #878:
URL: https://github.com/apache/pulsar-client-go/pull/878#discussion_r1010199707
##########
pulsar/producer_partition.go:
##########
@@ -1035,9 +1035,15 @@ func (p *partitionProducer) Send(ctx context.Context, msg *ProducerMessage) (Mes
}, true)
// wait for send request to finish
- <-doneCh
-
- return msgID, err
+ for {
Review Comment:
I also think this loop is useless.
--
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] Gleiphir2769 commented on pull request #878: [Issue 877]Fix send stuck
Posted by GitBox <gi...@apache.org>.
Gleiphir2769 commented on PR #878:
URL: https://github.com/apache/pulsar-client-go/pull/878#issuecomment-1297987240
> In the issue link have the stack. I think wait in a channel should with select timeout context to avoid stuck.
You are right, timeout context can avoid `doneCh` stuck.
But `send()` stuck shouldn't happen. I think add select just skip this problem.
--
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] Gleiphir2769 commented on a diff in pull request #878: [Issue 877]Fix send stuck
Posted by GitBox <gi...@apache.org>.
Gleiphir2769 commented on code in PR #878:
URL: https://github.com/apache/pulsar-client-go/pull/878#discussion_r1010049089
##########
pulsar/producer_partition.go:
##########
@@ -1035,9 +1035,15 @@ func (p *partitionProducer) Send(ctx context.Context, msg *ProducerMessage) (Mes
}, true)
// wait for send request to finish
- <-doneCh
-
- return msgID, err
+ for {
Review Comment:
```suggestion
```
Hi, I don't think a loop is needed here.
--
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] Gleiphir2769 commented on pull request #878: [Issue 877]Fix send stuck
Posted by GitBox <gi...@apache.org>.
Gleiphir2769 commented on PR #878:
URL: https://github.com/apache/pulsar-client-go/pull/878#issuecomment-1297116137
Hi, could you upload the full error stack? I think we need to find out real reason for doneCh can not be closed. Because doneCh is never closed means that the callback of `sendRequest` is not be called. This shouldn't happen.
--
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] billowqiu commented on pull request #878: [Issue 877]Fix send stuck
Posted by GitBox <gi...@apache.org>.
billowqiu commented on PR #878:
URL: https://github.com/apache/pulsar-client-go/pull/878#issuecomment-1297267318
> Hi, could you upload the full error stack? I think we need to find out the real reason for doneCh can not be closed. Because doneCh is never closed means that the callback of `sendRequest` is not be called. This shouldn't happen.
In the issue link have the stack. I think wait in a channel should with select timeout context to avoid stuck.
--
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] RobertIndie commented on pull request #878: [Issue 877]Fix send stuck
Posted by "RobertIndie (via GitHub)" <gi...@apache.org>.
RobertIndie commented on PR #878:
URL: https://github.com/apache/pulsar-client-go/pull/878#issuecomment-1630305995
Fixed by https://github.com/apache/pulsar-client-go/pull/1053
--
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