Skip to content

metadata/gc: remove the noop-loop for snapshot reference#2963

Merged
estesp merged 1 commit intocontainerd:masterfrom
fuweid:remove-noop-loop
Jan 29, 2019
Merged

metadata/gc: remove the noop-loop for snapshot reference#2963
estesp merged 1 commit intocontainerd:masterfrom
fuweid:remove-noop-loop

Conversation

@fuweid
Copy link
Copy Markdown
Member

@fuweid fuweid commented Jan 29, 2019

The noop-loop does nothing.

The containerd doesn't have any snapshotter buckets at the beginning.
If user uses specific dir as rootfs to create container, and sets
snapshotter key to the container, like overlayfs by mistake,
the gc scheduler will try to scan the snapshotter and panic.

In order to avoid this case, remove the noop-loop here.

Signed-off-by: Wei Fu [email protected]

for example,

image

The noop-loop does nothing.

The containerd doesn't have any snapshotter buckets at the beginning.
If user uses specific dir as rootfs to create container, and sets
`snapshotter` key to the container, like `overlayfs` by mistake,
the gc scheduler will try to scan the snapshotter and panic.

In order to avoid this case, remove the noop-loop here.

Signed-off-by: Wei Fu <[email protected]>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jan 29, 2019

Codecov Report

Merging #2963 into master will decrease coverage by 3.64%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2963      +/-   ##
==========================================
- Coverage   47.62%   43.98%   -3.65%     
==========================================
  Files          92      102      +10     
  Lines        8517    10873    +2356     
==========================================
+ Hits         4056     4782     +726     
- Misses       3734     5358    +1624     
- Partials      727      733       +6
Flag Coverage Δ
#linux 47.63% <ø> (+0.01%) ⬆️
#windows 41.19% <ø> (?)
Impacted Files Coverage Δ
metadata/gc.go 61.51% <ø> (-4.05%) ⬇️
snapshots/native/native.go 43.04% <0%> (-9.99%) ⬇️
metadata/snapshot.go 45.8% <0%> (-8.96%) ⬇️
archive/tar.go 43.79% <0%> (-7.07%) ⬇️
metadata/containers.go 47.97% <0%> (-6.62%) ⬇️
content/local/writer.go 57.84% <0%> (-6.36%) ⬇️
content/local/store.go 48.51% <0%> (-5.03%) ⬇️
archive/tar_opts.go 28.57% <0%> (-4.77%) ⬇️
archive/compression/compression.go 58.69% <0%> (-4.7%) ⬇️
metadata/images.go 58.46% <0%> (-4.7%) ⬇️
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f35c352...34672d4. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit e26f442 into containerd:master Jan 29, 2019
@dmcgowan
Copy link
Copy Markdown
Member

All buckets should definitely be nil checked before used. Since this condition can arrive based on specific data input and it is a noop, we should probably just back port to 1.1 and 1.2

@fuweid fuweid deleted the remove-noop-loop branch January 30, 2019 01:54
@fuweid
Copy link
Copy Markdown
Member Author

fuweid commented Jan 30, 2019

hi @dmcgowan , it is reasonable. I make two prs for the backport

#2965
#2966

please take a look.

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.

6 participants