cckellogg commented on a change in pull request #535:
URL: https://github.com/apache/pulsar-client-go/pull/535#discussion_r648521135
##########
File path: pulsar/consumer_partition.go
##########
@@ -971,9 +972,9 @@ func (pc *partitionConsumer) grabConn() error {
pc.name = res.Response.ConsumerStatsResponse.GetConsumerName()
}
- pc.conn = res.Cnx
+ pc._setConn(res.Cnx)
pc.log.Info("Connected consumer")
- pc.conn.AddConsumeHandler(pc.consumerID, pc)
+ pc._getConn().AddConsumeHandler(pc.consumerID, pc)
Review comment:
There was change that moved the broker reconnect out of the events go
routine (https://github.com/apache/pulsar-client-go/pull/376/files). This is
now causing the data race issue.
The question is if there is a reconnection what should happen with the
pending events in the event channel? Right now they are processed using a
stale/closed connection. Even with this fix it's possible events will try to
get processed using a stale connection. Maybe that is ok but for me it makes
the code difficult to follow and reason about. Ideally, we could come up with a
cleaner way so we are never unintentionally using a stale/closed connection.
Thoughts?
--
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:
[email protected]
