Skip to content

fix(p2p): stop peer tracker gc ticker - #407

Open
loglapa wants to merge 1 commit into
celestiaorg:mainfrom
loglapa:fix-peer-tracker-ticker-223
Open

fix(p2p): stop peer tracker gc ticker#407
loglapa wants to merge 1 commit into
celestiaorg:mainfrom
loglapa:fix-peer-tracker-ticker-223

Conversation

@loglapa

@loglapa loglapa commented Aug 20, 2026

Copy link
Copy Markdown

Closes #223

Summary

  • stop the peer tracker garbage-collection ticker when gc exits
  • release ticker resources when the tracker context is canceled

Testing

  • go test ./...
  • go build ./...
  • go vet ./...
  • golangci-lint run --timeout 10m
  • go mod tidy -diff
  • git diff --check

@loglapa
loglapa marked this pull request as ready for review August 20, 2026 10:41
@loglapa
loglapa requested a review from a team as a code owner August 20, 2026 10:41
@loglapa
loglapa requested a review from rach-id August 20, 2026 10:41
@rach-id
rach-id enabled auto-merge (squash) August 20, 2026 11:11
@mergify

mergify Bot commented Aug 20, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@loglapa

loglapa commented Aug 21, 2026

Copy link
Copy Markdown
Author

The only failing check is the previously observed store.TestStore failure. A clean local rerun of go test ./store -count=1 passes, indicating a flaky failure unrelated to this peer-tracker change. I attempted to rerun the failed GitHub Actions job, but GitHub restricts reruns to repository admins; a maintainer needs to rerun run 32359311645.

Signed-off-by: Vladislav Lapin <v.lapin@scalablesolutions.io>
auto-merge was automatically disabled August 24, 2026 14:31

Head branch was pushed to by a user without write access

@loglapa
loglapa force-pushed the fix-peer-tracker-ticker-223 branch from b903b71 to abdcee1 Compare August 24, 2026 14:31
@loglapa

loglapa commented Aug 24, 2026

Copy link
Copy Markdown
Author

@Mergifyio queue

@mergify

mergify Bot commented Aug 24, 2026

Copy link
Copy Markdown

queue

☑️ Command disallowed due to command restrictions in the Mergify configuration.

Details
  • sender-permission >= write

@rach-id
rach-id enabled auto-merge (squash) August 24, 2026 14:35
@loglapa

loglapa commented Aug 24, 2026

Copy link
Copy Markdown
Author

Fresh CI on the rebased head confirms the prior store.TestStore failure is gone. The new failure is instead in upstream TestExchangeServer_partialRangeNotExpanded, added by #406 on main: its partial-range fixture has a nil underlying store and panics in Tail(). The already-approved #409 is the focused fix for exactly that fixture. This peer-tracker patch does not touch range handling; once #409 lands, rebasing/rerunning #407 should clear this blocker. I am keeping the changes separate to avoid pulling an unrelated test fix into this PR.

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.

p2p: peerTracker.gc leaks time.Ticker due to missing defer ticker.Stop(); please integrate leak static analyzer

2 participants