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

test: increase logs max-size #3259

Merged
merged 1 commit into from
Jul 31, 2024
Merged

test: increase logs max-size #3259

merged 1 commit into from
Jul 31, 2024

Conversation

xyz-li
Copy link
Contributor

@xyz-li xyz-li commented Jul 30, 2024

Fix: #3225

I think max-size is too small and log output is too quicklly. There is one situation, nerdctl rotate the logfile, then os.OpenFile will return an error. At last this test will fail.

    container_logs_test.go:243: assertion failed: true (true bool) != false (len(tailLogs) > linesPerFile bool): time="2024-07-30T03:32:03Z" level=fatal msg="failed to stat JSON log file stat /var/lib/nerdctl/1935db59/containers/nerdctl-test/a90c5603fa813567b71dca7fe84b3bf6aea4dd06dc9175c8091fdfd138a87d00/a90c5603fa813567b71dca7fe84b3bf6aea4dd06dc9175c8091fdfd138a87d00-json.log: no such file or directory"

@apostasie
Copy link
Contributor

LGTM. This is going to help.

There is one situation, nerdctl rotate the logfile, then os.OpenFile will return an error.

Can we fix it ^? Can you point to it in the codebase?

Thanks a lot @xyz-li ! This one has been driving me bonkers.

@xyz-li
Copy link
Contributor Author

xyz-li commented Jul 30, 2024

Can we fix it ^? Can you point to it in the codebase?

I don't think we can. As the max-size is too small and container output logs too quickly. There will always exist this situation.
I think the best way is to limit the minimum value of max-size.

Signed-off-by: xyz-li <hui0787411@163.com>
@xyz-li
Copy link
Contributor Author

xyz-li commented Jul 30, 2024

I have decreased the max-size and slowed down the log output.
It takes less CPU time. And have not failed for about 1000 times on my machine.
@apostasie

@fahedouch
Copy link
Member

const sampleJSONLog = `{"log":"A\n","stream":"stdout","time":"2024-04-11T12:01:09.800288974Z"}`
const linesPerFile = 2
defer base.Cmd("rm", "-f", containerName).Run()
base.Cmd("run", "-d", "--log-driver", "json-file",
"--log-opt", fmt.Sprintf("max-size=%d", len(sampleJSONLog)*linesPerFile),

I think max-size is too small and log output is too quicklly. There is one situation, startTail return with true, and nerdctl rotate the logfile, then openFileShareDelete will return an error. At last this test will fail.

How about change linesPerFile to 1000.

root@lh:/root/go/src/github.com/containerd/nerdctl# while true; do go test ./cmd/nerdctl -count 1 -run TestTailFollowRotateLogs; done
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.168s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.688s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.447s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	3.688s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.488s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.658s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.904s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.362s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.581s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.527s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.124s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.902s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.020s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.410s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.846s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.540s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.973s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.168s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.679s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.667s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.335s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.582s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.449s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.467s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.151s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.055s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.971s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.490s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.785s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.126s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.863s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.637s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.546s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.115s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.578s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.573s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.567s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	3.680s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	4.220s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.636s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.087s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	5.068s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	3.755s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.726s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.672s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.745s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.535s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.689s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.542s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.160s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.372s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.938s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.543s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.634s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.220s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.223s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.270s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.510s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.615s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	3.755s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.567s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.673s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.727s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.399s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.160s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.675s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.667s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	2.495s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	4.484s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.106s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.194s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	0.612s
ok  	github.com/containerd/nerdctl/v2/cmd/nerdctl	1.059s

thanks for the investigation guys LGTM. @xyz-li is it possible to introduce more accurate behavior in this PR #3200 like a retry mechanism before returning error from openFileShareDelete ? or may be we can catch when the event is done with fsnotify and then we execute the openFileShareDelete

Copy link
Member

@fahedouch fahedouch left a comment

Choose a reason for hiding this comment

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

Thanks

@fahedouch fahedouch changed the title test: increase max-size test: increase logs max-size Jul 31, 2024
@fahedouch fahedouch merged commit 02497ad into containerd:main Jul 31, 2024
19 checks passed
@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Jul 31, 2024
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.

TestTailFollowRotateLogs is still failing
4 participants