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

Repeated trace in log file with error #53

Open
adrianlzt opened this issue Apr 3, 2019 · 10 comments · Fixed by #65 or kubernetes/kubernetes#78465
Open

Repeated trace in log file with error #53

adrianlzt opened this issue Apr 3, 2019 · 10 comments · Fixed by #65 or kubernetes/kubernetes#78465
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@adrianlzt
Copy link

/kind bug

What steps did you take and what happened:
https://play.golang.com/p/6l8KNG_iOsM

Configuring klog to write to a file and logtostderr=false, the log file is openned and wrote three times with the same content.

Removing the flush, writes three times the headers, but no content.

What did you expect to happen:
Only one open and write.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Apr 3, 2019
@DirectXMan12
Copy link

/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Apr 3, 2019
@adrianlzt
Copy link
Author

https://github.com/kubernetes/klog/blob/master/klog.go#L926

	// Files are created in decreasing severity order, so as soon as we find one
	// has already been created, we can stop.
	for s := sev; s >= infoLog && l.file[s] == nil; s-- {

This is going to create a new file for each severity. Opening three times (error, warning and info) the same file if we have defined a particular log_file.

Not sure why the comment says that it is going to stop.

@DirectXMan12
Copy link

it will stop if the files for a particular verbosity already exists (e.g. l.file[s] != nil) -- it just doesn't have a good definition of "already created". We should probably fix this somewhere though.

@tallclair
Copy link
Member

It's also configured to write to multiple files:

klog/klog.go

Lines 765 to 777 in e88f730

switch s {
case fatalLog:
l.file[fatalLog].Write(data)
fallthrough
case errorLog:
l.file[errorLog].Write(data)
fallthrough
case warningLog:
l.file[warningLog].Write(data)
fallthrough
case infoLog:
l.file[infoLog].Write(data)
}

@liggitt
Copy link
Member

liggitt commented Jun 12, 2019

/reopen

the attempted fix for this caused severe performance regressions

@k8s-ci-robot k8s-ci-robot reopened this Jun 12, 2019
@k8s-ci-robot
Copy link

@liggitt: Reopened this issue.

In response to this:

/reopen

the attempted fix for this caused severe performance regressions

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@liggitt
Copy link
Member

liggitt commented Jun 12, 2019

fix was reverted in #69

@liggitt
Copy link
Member

liggitt commented Jun 12, 2019

When a new fix is developed for this issue, it must be accompanied by (or preceded by) benchmark tests that exercise the cases that regressed in kubernetes/kubernetes#78465

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 18, 2019
@justaugustus
Copy link
Member

Any update on this?
/remove-lifecycle stale
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
7 participants