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

special case no sent/received message in network health check #1263

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

ceyonur
Copy link
Contributor

@ceyonur ceyonur commented Mar 29, 2023

Why this should be merged

lastSend and lastReceived time values default to 0, which would show errors in health check like:

{
  "network": {
    "message": {
      "connectedPeers": 0,
      "sendFailRate": 0,
      "timeSinceLastMsgReceived": "466123h20m46.862257771s",
      "timeSinceLastMsgSent": "466123h20m46.862257771s"
    },
    "error": "network layer is unhealthy reason: not connected to a minimum of 1 peer(s) only 0, no messages from network received in 466123h20m46.862257771s > 1m0s, no messages from network sent in 466123h20m46.862257771s > 1m0s",
    "timestamp": "2023-03-05T19:20:46.862278635Z",
    "duration": 56961,
    "contiguousFailures": 1,
    "timeOfFirstFailure": "2023-03-05T19:20:46.862278635Z"
  }
}

466123h20m46.862257771s > 1m0s seems a bit off.

This PR handles this case, so that we won't show time.Now in timeSinceLastMsgReceived/Sent and show them like:

"network": {
    "message": {
        "connectedPeers": 0,
        "sendFailRate": 0
    },
    "error": "network layer is unhealthy reason: not connected to a minimum of 1 peer(s) only 0, no messages received from network, no messages sent to network",
    "timestamp": "2023-03-29T14:04:08.258799+03:00",
    "duration": 104750,
    "contiguousFailures": 1,
    "timeOfFirstFailure": "2023-03-29T14:04:08.258799+03:00"
},

How this works

Special cases the case where last send/received = 0 in network health function. It also puts storing/reading these values into functions.

How this was tested

Locally tested

@ceyonur ceyonur self-assigned this Mar 29, 2023
@ceyonur ceyonur added the networking This involves networking label Mar 29, 2023
@StephenButtolph StephenButtolph added the monitoring This primarily focuses on logs, metrics, and/or tracing label Mar 29, 2023
Copy link
Contributor

@StephenButtolph StephenButtolph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small readability suggestions

network/network.go Outdated Show resolved Hide resolved
network/network.go Outdated Show resolved Hide resolved
@StephenButtolph StephenButtolph added this to the v1.10.0 (Cortina) milestone Mar 30, 2023
@joshua-kim joshua-kim added the bug Something isn't working label Mar 30, 2023
@StephenButtolph StephenButtolph merged commit 705c3fa into dev Mar 30, 2023
@StephenButtolph StephenButtolph deleted the fix-last-network-times branch March 30, 2023 15:51
hexfusion pushed a commit to hexfusion/avalanchego that referenced this pull request Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working monitoring This primarily focuses on logs, metrics, and/or tracing networking This involves networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants