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

Prevent races counting missed heartbeats #49882

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Conversation

rosstimothy
Copy link
Contributor

Increments the heartbeatsMissedByAuth gauge directly instead of keeping a local count and setting the gauge once. This fixes a race caught in https://github.com/gravitational/teleport/actions/runs/11828850190/job/32959672995?pr=48949.

==================
WARNING: DATA RACE
Write at 0x00c003857da8 by goroutine 48258:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations.func3.1()
      /__w/teleport/teleport/lib/auth/auth.go:1471 +0x84
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).getRange.func1.1()
      /__w/teleport/teleport/lib/services/unified_resource.go:239 +0x14d
  github.com/google/btree.(*node[go.shape.*uint8]).iterate()
      /go/pkg/mod/github.com/google/btree@v1.1.3/btree_generic.go:522 +0x62c
  github.com/google/btree.(*BTreeG[go.shape.*uint8]).AscendRange()
      /go/pkg/mod/github.com/google/btree@v1.1.3/btree_generic.go:752 +0x104
  github.com/google/btree.(*BTreeG[*github.com/gravitational/teleport/lib/services.item]).AscendRange-fm()
      <autogenerated>:1 +0x58
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).getRange.func1()
      /__w/teleport/teleport/lib/services/unified_resource.go:230 +0x7a8
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).read()
      /__w/teleport/teleport/lib/services/unified_resource.go:591 +0x2ad
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).getRange()
      /__w/teleport/teleport/lib/services/unified_resource.go:215 +0x2d9
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).IterateUnifiedResources()
      /__w/teleport/teleport/lib/services/unified_resource.go:288 +0xe6
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations.func3()
      /__w/teleport/teleport/lib/auth/auth.go:1464 +0x237

Previous read at 0x00c003857da8 by goroutine 48219:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations.func3()
      /__w/teleport/teleport/lib/auth/auth.go:1489 +0x2d2

Goroutine 48258 (running) created at:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations()
      /__w/teleport/teleport/lib/auth/auth.go:1460 +0xf9e
  github.com/gravitational/teleport/lib/auth.initCluster.gowrap1()
      /__w/teleport/teleport/lib/auth/init.go:594 +0x33

Goroutine 48219 (finished) created at:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations()
      /__w/teleport/teleport/lib/auth/auth.go:1460 +0xf9e
  github.com/gravitational/teleport/lib/auth.initCluster.gowrap1()
      /__w/teleport/teleport/lib/auth/init.go:594 +0x33
==================

Increments the `heartbeatsMissedByAuth` gauge directly instead of
keeping a local count and setting the gauge once. This fixes a
race caught in https://github.com/gravitational/teleport/actions/runs/11828850190/job/32959672995?pr=48949.

```
==================
WARNING: DATA RACE
Write at 0x00c003857da8 by goroutine 48258:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations.func3.1()
      /__w/teleport/teleport/lib/auth/auth.go:1471 +0x84
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).getRange.func1.1()
      /__w/teleport/teleport/lib/services/unified_resource.go:239 +0x14d
  github.com/google/btree.(*node[go.shape.*uint8]).iterate()
      /go/pkg/mod/github.com/google/btree@v1.1.3/btree_generic.go:522 +0x62c
  github.com/google/btree.(*BTreeG[go.shape.*uint8]).AscendRange()
      /go/pkg/mod/github.com/google/btree@v1.1.3/btree_generic.go:752 +0x104
  github.com/google/btree.(*BTreeG[*github.com/gravitational/teleport/lib/services.item]).AscendRange-fm()
      <autogenerated>:1 +0x58
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).getRange.func1()
      /__w/teleport/teleport/lib/services/unified_resource.go:230 +0x7a8
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).read()
      /__w/teleport/teleport/lib/services/unified_resource.go:591 +0x2ad
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).getRange()
      /__w/teleport/teleport/lib/services/unified_resource.go:215 +0x2d9
  github.com/gravitational/teleport/lib/services.(*UnifiedResourceCache).IterateUnifiedResources()
      /__w/teleport/teleport/lib/services/unified_resource.go:288 +0xe6
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations.func3()
      /__w/teleport/teleport/lib/auth/auth.go:1464 +0x237

Previous read at 0x00c003857da8 by goroutine 48219:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations.func3()
      /__w/teleport/teleport/lib/auth/auth.go:1489 +0x2d2

Goroutine 48258 (running) created at:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations()
      /__w/teleport/teleport/lib/auth/auth.go:1460 +0xf9e
  github.com/gravitational/teleport/lib/auth.initCluster.gowrap1()
      /__w/teleport/teleport/lib/auth/init.go:594 +0x33

Goroutine 48219 (finished) created at:
  github.com/gravitational/teleport/lib/auth.(*Server).runPeriodicOperations()
      /__w/teleport/teleport/lib/auth/auth.go:1460 +0xf9e
  github.com/gravitational/teleport/lib/auth.initCluster.gowrap1()
      /__w/teleport/teleport/lib/auth/init.go:594 +0x33
==================
```
@rosstimothy rosstimothy enabled auto-merge December 6, 2024 18:59
@rosstimothy rosstimothy added the no-changelog Indicates that a PR does not require a changelog entry label Dec 6, 2024
@rosstimothy rosstimothy added this pull request to the merge queue Dec 6, 2024
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from Tener December 6, 2024 19:16
Merged via the queue into master with commit ca04ea3 Dec 6, 2024
46 of 47 checks passed
@rosstimothy rosstimothy deleted the tross/missed_hb_race branch December 6, 2024 19:35
@public-teleport-github-review-bot

@rosstimothy See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Create PR
branch/v16 Create PR
branch/v17 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants