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

Back port some stable patches for v0.5.x #329

Merged
merged 2 commits into from
Jan 19, 2023

Conversation

changweige
Copy link
Member

Clearly pick some stable patches for v0.5 and prepare to release v0.5.1

There is a race that multiple goroutines can all WaitUntilState()
at the same time and waiting for the same expected state. So they
can both escape from the very beginning state check ending up with
an extra event record.

Signed-off-by: Changwei Ge <gechangwei@bytedance.com>
go race detected
==================
WARNING: DATA RACE
Read at 0x00c0000bed48 by goroutine 34:
  github.com/containerd/nydus-snapshotter/pkg/metrics/collector.(*DaemonInfoCollector).Collect()
      /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/metrics/collector/daemon.go:33 +0x78
  github.com/containerd/nydus-snapshotter/pkg/manager.(*Manager).handleDaemonDeathEvent()
      /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/manager/manager.go:314 +0x212
  github.com/containerd/nydus-snapshotter/pkg/manager.NewManager.func1()
      /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/manager/manager.go:364 +0x39

Previous write at 0x00c0000bed48 by goroutine 88:
  github.com/containerd/nydus-snapshotter/pkg/manager.(*Manager).StartDaemon.func2()
      /home/runner/work/nydus-snapshotter/nydus-snapshotter/pkg/manager/daemon_adaptor.go:97 +0x5d3

Signed-off-by: Changwei Ge <gechangwei@bytedance.com>
@changweige changweige changed the title Pick 0.5 Back port some stable patches for v0.5.x Jan 19, 2023
d.Lock()
collector.NewDaemonInfoCollector(&d.Version, -1).Collect()
Copy link
Member

Choose a reason for hiding this comment

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

Whether is a lock needed before another call to this method?

Copy link
Member

Choose a reason for hiding this comment

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

In a few cases, I get the extra values that d.version is incorrect. However, the value of the expected version is eventually correct.

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean by "incorrect"? The version string is shorter than expected or polluted?

Copy link
Member

Choose a reason for hiding this comment

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

The version string is null. It appeared by chance.

@changweige changweige merged commit 45b476a into containerd:stable/v0.5 Jan 19, 2023
@changweige changweige deleted the pick-0.5 branch February 6, 2023 06:18
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