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.

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

Fix bug: Agent never update its Instance HeartbeatTime #3167

Merged
merged 1 commit into from
Jul 25, 2019
Merged

Fix bug: Agent never update its Instance HeartbeatTime #3167

merged 1 commit into from
Jul 25, 2019

Conversation

evanxuhe
Copy link
Contributor

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • Related issues


Bug fix

  • Bug description.
    Agent never update its Instance HeartbeatTime

Beacause the judgment statement in org.apache.skywalking.apm.agent.core.remote.AppAndServiceRegisterClientthat compares current timestamp with lastSegmentUpdateTime will be always negative and never works

if (lastSegmentTime - System.currentTimeMillis() > 60 * 1000) {
                                instanceDiscoveryServiceBlockingStub.heartbeat(ApplicationInstanceHeartbeat.newBuilder()
                                    .setApplicationInstanceId(RemoteDownstreamConfig.Agent.APPLICATION_INSTANCE_ID)
                                    .setHeartbeatTime(System.currentTimeMillis())
                                    .build());
                            }
  • How to fix?
    Fix the judgement sequence putting System.currentTimeMillis() before
                            if ( System.currentTimeMillis() - lastSegmentTime > 60 * 1000) {
                                instanceDiscoveryServiceBlockingStub.heartbeat(ApplicationInstanceHeartbeat.newBuilder()
                                    .setApplicationInstanceId(RemoteDownstreamConfig.Agent.APPLICATION_INSTANCE_ID)
                                    .setHeartbeatTime(System.currentTimeMillis())
                                    .build());
                            }

New feature or improvement

  • Describe the details and related test reports.

@wu-sheng
Copy link
Member

Is this only a 5.x bug? Or still exist in master 5.x protocol?

@evanxuhe
Copy link
Contributor Author

evanxuhe commented Jul 25, 2019

relavant disccussion before
#3150

@wu-sheng wu-sheng merged commit 27790b9 into apache:5.x Jul 25, 2019
@evanxuhe
Copy link
Contributor Author

Is this only a 5.x bug? Or still exist in master 5.x protocol?

I think it exists in 5.x master agent
I dont understand what you wanna know? . 5.x protocol Does it be relevant with this?

@wu-sheng
Copy link
Member

No related. Misread from me. Sorry.

@wu-sheng wu-sheng added this to the 5.1.0 milestone Jul 25, 2019
bootsrc pushed a commit to bootsrc/skywalking that referenced this pull request Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants