Skip to content

Revised oomparser to not use all the cpu#549

Merged
vmarmol merged 1 commit into
google:masterfrom
kateknister:master
Mar 3, 2015
Merged

Revised oomparser to not use all the cpu#549
vmarmol merged 1 commit into
google:masterfrom
kateknister:master

Conversation

@kateknister

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread utils/oomparser/oomparser.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we know if watching is better? For many systems, /var/log/messages gets updated more than once per second.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you mean there might be a backlog of IN_MODIFY notices to the watcher if there are many changes to the file all at once?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/var/log/messages gets debug logs from many sources. The watcher might get invoked more than once per second.

Were we doing a periodic poll earlier? Can we just check once every 10s and keep an open fd?
(Sorry, I haven't looked at all details of oom-parser. But I feel like watching might not help some of the machines).

@kateknister

Copy link
Copy Markdown
Contributor Author

Oomparser on with 100ms delay for every loop call: (average: 0.010 cores), 9.785 MB of memory

No oomparser: (average: 0.008 cores), 9.753 MB of memory

The delay minimizes the effect of the oomparser but there is a bit of difference.

Comment thread utils/oomparser/oomparser.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we move this logic to analyzeLines instead?

@kateknister kateknister force-pushed the master branch 3 times, most recently from fb3d206 to b671a79 Compare February 28, 2015 00:56
Comment thread utils/oomparser/oomparser.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code is getting a bit hard to read, let's try to simplify it. We will never break out of this loop so lets make it an infinite loop with a read at the beginning. We should also only wait while the error is EOF:

var line string
var err error
for true {
    // Wait for more content to show up in the file.
    for line, err = ioreader.ReadString('\n'); err != nil && err == io.EOF; {
        time.Sleep(100 * time.Millisecond)
    }

    // Do work...
}

@vmarmol

vmarmol commented Mar 3, 2015

Copy link
Copy Markdown
Contributor

LGTM

vmarmol added a commit that referenced this pull request Mar 3, 2015
Revised oomparser to not use all the cpu
@vmarmol vmarmol merged commit 5e6c11a into google:master Mar 3, 2015
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.

3 participants