You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2017/08/25 21:35:43 UTC
[kudu-CR] KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
Hao Hao has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7835
Change subject: KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
......................................................................
KUDU-2102: fix PosixRWFile::Sync to guarantee durability when
used concurrently
PosixRWFile has an AtomicBool to optimize the perfomance of Sync()
so that when two threads call Sync() at the same time, only one will
actually do the fsync(). However, the optimazation may results in one
thread returning from the call early and operating under the assumption
that the file's data has been made durable, even though it is stil in
the process of being synchronized to disk.
This patch fixs this issue simply by removing the optimization. Since
the overhead of no-op fsync() is purely syscall-related.
Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
---
M src/kudu/util/env_posix.cc
1 file changed, 0 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/7835/1
--
To view, visit http://gerrit.cloudera.org:8080/7835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
[kudu-CR] KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
......................................................................
Patch Set 1:
(5 comments)
It looks like IWYU is giving false alarm. Maybe we can ignore it again?
http://gerrit.cloudera.org:8080/#/c/7835/1//COMMIT_MSG
Commit Message:
PS1, Line 12: results
> result
Done
PS1, Line 12: optimazation
> optimization
Done
PS1, Line 14: stil
> still
Done
PS1, Line 17: fixs
> fixes
Done
PS1, Line 17: Since
: the overhead of no-op fsync() is purely syscall-related
> This reads like a sentence fragment. Maybe you're missing the second part o
Done
--
To view, visit http://gerrit.cloudera.org:8080/7835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7835
to look at the new patch set (#2).
Change subject: KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
......................................................................
KUDU-2102: fix PosixRWFile::Sync to guarantee durability when
used concurrently
PosixRWFile has an AtomicBool to optimize the perfomance of Sync()
so that when two threads call Sync() at the same time, only one will
actually do the fsync(). However, the optimization may result in one
thread returning from the call early and operating under the assumption
that the file's data has been made durable, even though it is still in
the process of being synchronized to disk.
This patch fixes this issue simply by removing the optimization. Since
the overhead of no-op fsync() is purely syscall-related, this optimization
wasn't meaningful to begin with.
Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
---
M src/kudu/util/env_posix.cc
1 file changed, 0 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/7835/2
--
To view, visit http://gerrit.cloudera.org:8080/7835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7835/1//COMMIT_MSG
Commit Message:
PS1, Line 12: optimazation
optimization
PS1, Line 12: results
result
PS1, Line 14: stil
still
PS1, Line 17: fixs
fixes
PS1, Line 17: Since
: the overhead of no-op fsync() is purely syscall-related
This reads like a sentence fragment. Maybe you're missing the second part of the sentence? "Since the overhead of no-op fsync() is purely syscall related, this optimization wasn't meaningful to begin with."
--
To view, visit http://gerrit.cloudera.org:8080/7835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
......................................................................
KUDU-2102: fix PosixRWFile::Sync to guarantee durability when
used concurrently
PosixRWFile has an AtomicBool to optimize the perfomance of Sync()
so that when two threads call Sync() at the same time, only one will
actually do the fsync(). However, the optimization may result in one
thread returning from the call early and operating under the assumption
that the file's data has been made durable, even though it is still in
the process of being synchronized to disk.
This patch fixes this issue simply by removing the optimization. Since
the overhead of no-op fsync() is purely syscall-related, this optimization
wasn't meaningful to begin with.
Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Reviewed-on: http://gerrit.cloudera.org:8080/7835
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/util/env_posix.cc
1 file changed, 0 insertions(+), 8 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/7835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
[kudu-CR] KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-2102: fix PosixRWFile::Sync to guarantee durability when used concurrently
......................................................................
Patch Set 2: Verified+1
Another false positive from IWYU, the rest of the build succeeded though.
--
To view, visit http://gerrit.cloudera.org:8080/7835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifaf2686233b2553de6d4a4c76fa67cc48dd340d4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No