RongtongJin commented on code in PR #1270:
URL: https://github.com/apache/rocketmq-clients/pull/1270#discussion_r3394846296
##########
golang/client.go:
##########
@@ -144,8 +144,7 @@ func (cs *defaultClientSession) startUp() {
// we assume that the list of the servers
hasn't changed, so the server that sent the message is still present.
hearbeat_response, err :=
cs.cli.clientManager.HeartBeat(context.TODO(), cs.endpoints,
&v2.HeartbeatRequest{}, 10*time.Second)
if err == nil && hearbeat_response.Status.Code
== v2.Code_OK {
- cs.cli.log.Info("Managed to recover,
executing message")
-
cs._execute_server_telemetry_command(response)
+ cs.cli.log.Info("Managed to recover")
Review Comment:
This log message change breaks the existing
`TestRestoreDefaultClientSessionOneError`, which still expects `"Managed to
recover, executing message"`. Please update the test expectation, and ideally
assert the command is executed exactly once so this PR's intended behavior is
covered directly.
##########
golang/client.go:
##########
@@ -144,8 +144,7 @@ func (cs *defaultClientSession) startUp() {
// we assume that the list of the servers
hasn't changed, so the server that sent the message is still present.
hearbeat_response, err :=
cs.cli.clientManager.HeartBeat(context.TODO(), cs.endpoints,
&v2.HeartbeatRequest{}, 10*time.Second)
if err == nil && hearbeat_response.Status.Code
== v2.Code_OK {
- cs.cli.log.Info("Managed to recover,
executing message")
-
cs._execute_server_telemetry_command(response)
+ cs.cli.log.Info("Managed to recover")
} else {
cs.cli.log.Errorf("Failed to recover,
Some of the servers are unhealthy, Heartbeat err=%w", err)
cs.release()
Review Comment:
After this failed recovery path calls `release()`, execution still falls
through to the unconditional `_execute_server_telemetry_command(response)`
below. That means a command received while recovering can still be executed
even though the heartbeat check failed. Please `continue` or otherwise skip
command execution in this branch after releasing the observer.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
