You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/06/08 22:50:02 UTC
[kudu-CR](branch-1.4.x) log block manager: fix corruption after re-opening compacted metadata
Hello Adar Dembo, Kudu Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/7125
to review the following change.
Change subject: log_block_manager: fix corruption after re-opening compacted metadata
......................................................................
log_block_manager: fix corruption after re-opening compacted metadata
This fixes an issue discovered on a cluster due to the following
sequence of events:
- a block manager compacts a metadata file while starting up
- when it reopens the metadata file after replacing it with the
compacted one, it gets a file_cache hit. Thus, the WritablePBContainer
continues to write to the _deleted_ file instead of the compacted one.
Metadata entries at this point are lost (which could cause block loss
in the case of lost CREATE records, or dangling blocks in the case of
lost DELETEs)
- if the server continues to run for a while, the FD will be evicted
from the cache and eventually re-opened. At that point, a further
DELETE record could end up writing to an offset past the end of the
file, since the write offset was incremented by the "lost" records
above.
- on the next restart, the metadata file would have a "gap" of zero
bytes, which would surface as a checksum failure and failure to start
up.
The fix is relatively simple: when we replace the metadata file we need
to invalidate and evict the cache entry so that when we "reopen", it
actually starts appending to the _new_ file and not the old deleted one.
The bulk of the changes here are to tests:
- the stress test now enforces a minimum number of live blocks before it
starts deleting them. It also more aggressively compacts, and has a
smaller cache. With these changes, I was sometimes able to reproduce
the issue.
- A more targeted test issues a canned sequence of block creations and
deletions that can reliably reproduce the above issue.
Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Reviewed-on: http://gerrit.cloudera.org:8080/7113
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/file_cache-test.cc
M src/kudu/util/file_cache.cc
M src/kudu/util/file_cache.h
6 files changed, 216 insertions(+), 64 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/7125/1
--
To view, visit http://gerrit.cloudera.org:8080/7125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.4.x) log block manager: fix corruption after re-opening compacted metadata
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Adar Dembo, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7125
to look at the new patch set (#2).
Change subject: log_block_manager: fix corruption after re-opening compacted metadata
......................................................................
log_block_manager: fix corruption after re-opening compacted metadata
This fixes an issue discovered on a cluster due to the following
sequence of events:
- a block manager compacts a metadata file while starting up
- when it reopens the metadata file after replacing it with the
compacted one, it gets a file_cache hit. Thus, the WritablePBContainer
continues to write to the _deleted_ file instead of the compacted one.
Metadata entries at this point are lost (which could cause block loss
in the case of lost CREATE records, or dangling blocks in the case of
lost DELETEs)
- if the server continues to run for a while, the FD will be evicted
from the cache and eventually re-opened. At that point, a further
DELETE record could end up writing to an offset past the end of the
file, since the write offset was incremented by the "lost" records
above.
- on the next restart, the metadata file would have a "gap" of zero
bytes, which would surface as a checksum failure and failure to start
up.
The fix is relatively simple: when we replace the metadata file we need
to invalidate and evict the cache entry so that when we "reopen", it
actually starts appending to the _new_ file and not the old deleted one.
The bulk of the changes here are to tests:
- the stress test now enforces a minimum number of live blocks before it
starts deleting them. It also more aggressively compacts, and has a
smaller cache. With these changes, I was sometimes able to reproduce
the issue.
- A more targeted test issues a canned sequence of block creations and
deletions that can reliably reproduce the above issue.
Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Reviewed-on: http://gerrit.cloudera.org:8080/7113
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/file_cache-test.cc
M src/kudu/util/file_cache.cc
M src/kudu/util/file_cache.h
6 files changed, 216 insertions(+), 64 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/7125/2
--
To view, visit http://gerrit.cloudera.org:8080/7125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR](branch-1.4.x) log block manager: fix corruption after re-opening compacted metadata
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: log_block_manager: fix corruption after re-opening compacted metadata
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR](branch-1.4.x) log block manager: fix corruption after re-opening compacted metadata
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: log_block_manager: fix corruption after re-opening compacted metadata
......................................................................
Patch Set 1:
Had to resolve a tiny conflict in block_manager-stress-test (branch-1.4 doesn't have the LoadDataDirs stuff from master)
--
To view, visit http://gerrit.cloudera.org:8080/7125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR](branch-1.4.x) log block manager: fix corruption after re-opening compacted metadata
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: log_block_manager: fix corruption after re-opening compacted metadata
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR](branch-1.4.x) log block manager: fix corruption after re-opening compacted metadata
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: log_block_manager: fix corruption after re-opening compacted metadata
......................................................................
log_block_manager: fix corruption after re-opening compacted metadata
This fixes an issue discovered on a cluster due to the following
sequence of events:
- a block manager compacts a metadata file while starting up
- when it reopens the metadata file after replacing it with the
compacted one, it gets a file_cache hit. Thus, the WritablePBContainer
continues to write to the _deleted_ file instead of the compacted one.
Metadata entries at this point are lost (which could cause block loss
in the case of lost CREATE records, or dangling blocks in the case of
lost DELETEs)
- if the server continues to run for a while, the FD will be evicted
from the cache and eventually re-opened. At that point, a further
DELETE record could end up writing to an offset past the end of the
file, since the write offset was incremented by the "lost" records
above.
- on the next restart, the metadata file would have a "gap" of zero
bytes, which would surface as a checksum failure and failure to start
up.
The fix is relatively simple: when we replace the metadata file we need
to invalidate and evict the cache entry so that when we "reopen", it
actually starts appending to the _new_ file and not the old deleted one.
The bulk of the changes here are to tests:
- the stress test now enforces a minimum number of live blocks before it
starts deleting them. It also more aggressively compacts, and has a
smaller cache. With these changes, I was sometimes able to reproduce
the issue.
- A more targeted test issues a canned sequence of block creations and
deletions that can reliably reproduce the above issue.
Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Reviewed-on: http://gerrit.cloudera.org:8080/7113
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/7125
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/file_cache-test.cc
M src/kudu/util/file_cache.cc
M src/kudu/util/file_cache.h
6 files changed, 216 insertions(+), 64 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7125
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I491eacbad4750efedea854a2cc35b8ec994f9077
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>