You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2016/11/09 23:03:06 UTC
[kudu-CR] log block manager: fix when append exceeds preallocated space
Hello Jean-Daniel Cryans,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5024
to review the following change.
Change subject: log block manager: fix when append exceeds preallocated space
......................................................................
log block manager: fix when append exceeds preallocated space
In commit abea8c6 I added a DCHECK to enforce the assumption that a
container's preallocation offset is always ahead of its append offset. This
turns out to be wrong when very large values are written, or when the
preallocation size is tuned down (as it is in disk_reservation-itest.cc).
The effect: disk_reservation-itest.cc failed from time to time because the
tserver crashed too early.
So let's throw that away, and let's also adjust the preallocation algorithm
slightly: when we preallocate, we take the write offset into account. If
it's greater than the preallocation offset, we preallocate again, and we do
so from the write offset.
The new test crashes 100% of the time without the fix.
Change-Id: I99460d35e355abf843c965d3c2b4309cba24eadd
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
2 files changed, 29 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/5024/1
--
To view, visit http://gerrit.cloudera.org:8080/5024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I99460d35e355abf843c965d3c2b4309cba24eadd
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
[kudu-CR] log block manager: fix when append exceeds preallocated space
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: log block manager: fix when append exceeds preallocated space
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I99460d35e355abf843c965d3c2b4309cba24eadd
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] log block manager: fix when append exceeds preallocated space
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: log block manager: fix when append exceeds preallocated space
......................................................................
log block manager: fix when append exceeds preallocated space
In commit abea8c6 I added a DCHECK to enforce the assumption that a
container's preallocation offset is always ahead of its append offset. This
turns out to be wrong when very large values are written, or when the
preallocation size is tuned down (as it is in disk_reservation-itest.cc).
The effect: disk_reservation-itest.cc failed from time to time because the
tserver crashed too early.
So let's throw that away, and let's also adjust the preallocation algorithm
slightly: when we preallocate, we take the write offset into account. If
it's greater than the preallocation offset, we preallocate again, and we do
so from the write offset.
The new test crashes 100% of the time without the fix.
Change-Id: I99460d35e355abf843c965d3c2b4309cba24eadd
Reviewed-on: http://gerrit.cloudera.org:8080/5024
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Tested-by: Kudu Jenkins
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
2 files changed, 29 insertions(+), 8 deletions(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I99460d35e355abf843c965d3c2b4309cba24eadd
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins