Skip to content

Fix readSyncBulkPayload().#7839

Merged
oranagra merged 1 commit intoredis:unstablefrom
trevor211:fixReadSyncBulkPayload
Sep 25, 2020
Merged

Fix readSyncBulkPayload().#7839
oranagra merged 1 commit intoredis:unstablefrom
trevor211:fixReadSyncBulkPayload

Conversation

@trevor211
Copy link
Contributor

We should sync temp DB file before renaming as rdb_fsync_range does not use
flag SYNC_FILE_RANGE_WAIT_AFTER.

Refer to Linux Programmer's Manual:
SYNC_FILE_RANGE_WAIT_AFTER
Wait upon write-out of all pages in the range after performing any write.

Comment on lines 1742 to 1751
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to put it between the rename and open (which is actually part of the rename. (also makes a shorter error handling)

Suggested change
/* Rename rdb like renaming rewrite aof asynchronously. */
int old_rdb_fd = open(server.rdb_filename,O_RDONLY|O_NONBLOCK);
if (fsync(server.repl_transfer_fd) == -1) {
serverLog(LL_WARNING,
"Failed trying to sync the temp DB to disk in "
"MASTER <-> REPLICA synchronization: %s",
strerror(errno));
cancelReplicationHandshake(1);
if (old_rdb_fd != -1) close(old_rdb_fd);
return;
}
/* Make sure the new file (also used for persistence) is fully synced
* (not covered by earlier calls to rdb_fsync_range). */
if (fsync(server.repl_transfer_fd) == -1) {
serverLog(LL_WARNING,
"Failed trying to sync the temp DB to disk in "
"MASTER <-> REPLICA synchronization: %s",
strerror(errno));
cancelReplicationHandshake(1);
return;
}
/* Rename rdb like renaming rewrite aof asynchronously. */
int old_rdb_fd = open(server.rdb_filename,O_RDONLY|O_NONBLOCK);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed and rebased manually.

We should sync temp DB file before renaming as rdb_fsync_range does not use
flag `SYNC_FILE_RANGE_WAIT_AFTER`.

Refer to `Linux Programmer's Manual`:
SYNC_FILE_RANGE_WAIT_AFTER
    Wait upon write-out of all pages in the range after performing any write.
@trevor211 trevor211 force-pushed the fixReadSyncBulkPayload branch from d8a3636 to 63794e3 Compare September 25, 2020 00:02
@oranagra oranagra merged commit 0d62caa into redis:unstable Sep 25, 2020
@trevor211 trevor211 deleted the fixReadSyncBulkPayload branch October 13, 2020 08:21
@oranagra oranagra mentioned this pull request Oct 26, 2020
oranagra pushed a commit that referenced this pull request Oct 27, 2020
We should sync temp DB file before renaming as rdb_fsync_range does not use
flag `SYNC_FILE_RANGE_WAIT_AFTER`.

Refer to `Linux Programmer's Manual`:
SYNC_FILE_RANGE_WAIT_AFTER
    Wait upon write-out of all pages in the range after performing any write.

(cherry picked from commit 0d62caa)
oranagra pushed a commit that referenced this pull request Oct 27, 2020
We should sync temp DB file before renaming as rdb_fsync_range does not use
flag `SYNC_FILE_RANGE_WAIT_AFTER`.

Refer to `Linux Programmer's Manual`:
SYNC_FILE_RANGE_WAIT_AFTER
    Wait upon write-out of all pages in the range after performing any write.

(cherry picked from commit 0d62caa)
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Nov 4, 2020
We should sync temp DB file before renaming as rdb_fsync_range does not use
flag `SYNC_FILE_RANGE_WAIT_AFTER`.

Refer to `Linux Programmer's Manual`:
SYNC_FILE_RANGE_WAIT_AFTER
    Wait upon write-out of all pages in the range after performing any write.
jschmieg pushed a commit to memKeyDB/memKeyDB that referenced this pull request Nov 6, 2020
We should sync temp DB file before renaming as rdb_fsync_range does not use
flag `SYNC_FILE_RANGE_WAIT_AFTER`.

Refer to `Linux Programmer's Manual`:
SYNC_FILE_RANGE_WAIT_AFTER
    Wait upon write-out of all pages in the range after performing any write.

(cherry picked from commit 0d62caa)
pulllock pushed a commit to pulllock/redis that referenced this pull request Jun 28, 2023
We should sync temp DB file before renaming as rdb_fsync_range does not use
flag `SYNC_FILE_RANGE_WAIT_AFTER`.

Refer to `Linux Programmer's Manual`:
SYNC_FILE_RANGE_WAIT_AFTER
    Wait upon write-out of all pages in the range after performing any write.

(cherry picked from commit 0d62caa)
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.

2 participants