MDEV - 38507 fadvise64() called on socat pipe by Mariabackup is useless overhead (ESPIPE)#4748
Open
kjarir wants to merge 1 commit intoMariaDB:10.11from
Open
MDEV - 38507 fadvise64() called on socat pipe by Mariabackup is useless overhead (ESPIPE)#4748kjarir wants to merge 1 commit intoMariaDB:10.11from
kjarir wants to merge 1 commit intoMariaDB:10.11from
Conversation
…s overhead (ESPIPE) Currently, posix_fadvise(..., 0, 0, POSIX_FADV_DONTNEED) is executed repeatedly after each my_write. In case of pipes, it evaluates to ESPIPE and fails. Because it executes after every write, this results in millions of redundant syscalls, introducing massive overhead. As proposed by Daniel Black, posix_fadvise with a length/size of 0 (which equates to EOF) should be invoked once when the file is opened, rather than repetitively after every write. This change moves the fadvise call to the respective _open functions for ds_local, ds_stdout and ds_tmpfile datasinks. It fails fast on pipes early on and eliminates the redundant syscall overhead for subsequent write operations.
de5a73b to
67268ac
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background:
When taking backups or streaming data through pipes using
mariabackup,posix_fadvisewith a 0 length/size was being called restrictively after every successful write operation across ds_stdout.cc, ds_tmpfile.cc, and ds_local.cc datasinks.In the case of pipes,
posix_fadviseevaluates toESPIPEand fails. Since it was being executed inside themy_writecondition logic repeatedly for every single write, it introduced massive overhead through millions of redundant, failing syscalls.Changes in this PR:
As discussed and suggested by @grooverdan in the issue thread, the
posix_fadvisecall with a length/size of 0 (which equates to EOF/entire file) only needs to be made once when the file/sink is opened, rather than repetitively after every write.We moved the respective
posix_fadvise(..., 0, 0, POSIX_FADV_DONTNEED)calls out ofmy_writeblocks into the initialization/open blocks (local_open, stdout_open, tmpfile_open).Impact:
posix_fadvisesystem call counts, improving performance.Testing done:
Verified the correctness of the changes by successfully running and passing the
mariabackuptest suite in a local build environment: