Skip to content

TS-5085 posix_fadvise is incorrectly used in traffic_logcat#1254

Merged
shinrich merged 1 commit intoapache:masterfrom
danobi:TS-5085
Jan 4, 2017
Merged

TS-5085 posix_fadvise is incorrectly used in traffic_logcat#1254
shinrich merged 1 commit intoapache:masterfrom
danobi:TS-5085

Conversation

@danobi
Copy link
Copy Markdown
Member

@danobi danobi commented Dec 7, 2016

traffic_logcat was not using posix_fadvise correctly. This
patch corrects the original poor usage as well as adds some more
posix_fadvise calls that should help with performance.

@jpeach
Copy link
Copy Markdown
Contributor

jpeach commented Dec 7, 2016

[approve ci]

}
// Now that we're done reading a potentially large log file, we can tell the kernel that it's OK to evict
// the associated log file pages from cache
posix_fadvise(in_fd, 0, 0, POSIX_FADV_DONTNEED);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops, I forgot to wrap this call in #if's. Will fix soon.

@atsci
Copy link
Copy Markdown

atsci commented Dec 8, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1255/ for details.

@atsci
Copy link
Copy Markdown

atsci commented Dec 8, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/1150/ for details.

@zwoop zwoop added the Logging label Dec 8, 2016
@zwoop zwoop added this to the 7.1.0 milestone Dec 8, 2016
@shinrich
Copy link
Copy Markdown
Member

Looks good to me. Can you squash?

`traffic_logcat` was not using `posix_fadvise` correctly. This
patch corrects the original poor usage as well as adds some more
`posix_fadvise` calls that should help with performance.
@danobi
Copy link
Copy Markdown
Member Author

danobi commented Dec 14, 2016

@shinrich Done.

@shinrich shinrich merged commit be24334 into apache:master Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants