You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2018/05/08 02:19:36 UTC
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10340
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
WIP KUDU-2359: allow startup with missing data dirs
This patch allows Kudu to start up with some of the expected data
directories missing. Note that this behavior is different than starting
up Kudu with extra data directories in `fs_data_dirs`, which is still
not supported unless running the update tool.
Some notes and changes in this patch include:
- methods involved in loading PathInstanceMetadataFiles will now treat
missing instance files as "unhealthy", the same way they treat
instance files that fail due disk errors
- DataDirManager::LoadInstances() has been updated to handle the above
change, optionally returning the missing instances as unhealthy
instances
- various codepaths that previously ended FsManager::Open() with an
IOError/Corruption because all drives were failed will now return
NotFound, indicating Kudu should attempt to create a new FS layout
- as a byproduct of the above changes, when opening the FS layout in
ENFORCE_CONSISTENCY mode with an extra data dir included in
`fs_data_dirs`, Kudu will fail later than before, at the integrity
check, and yield an IOError instead of a NotFound error
- the UUID and UUID index assignment for missing/failed directories has
been updated when opening the speculative directory manager; see
DataDirManager::Open() for more details.
WIP this change touches codepaths with a fair number of edge cases, and
as such, I haven't fully convinced myself this is the best approach.
That said, it is reviewable and has tests passing. Open to feedback.
Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/tools/kudu-tool-test.cc
7 files changed, 259 insertions(+), 126 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/10340/1
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@54
PS2, Line 54: that previously triggered disk failure handling would trigger that
> Nit: due to
Still open.
http://gerrit.cloudera.org:8080/#/c/10340/5/src/kudu/fs/block_manager_util.cc
File src/kudu/fs/block_manager_util.cc:
http://gerrit.cloudera.org:8080/#/c/10340/5/src/kudu/fs/block_manager_util.cc@56
PS5, Line 56: PIMFs
I don't think this acronym is used elsewhere in the code.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 23 May 2018 22:13:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10340
to look at the new patch set (#4).
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
KUDU-2359: allow startup with missing data dirs
Context
-------
As a part of previous disk failure work, Kudu currently supports opening
the FS layout in the face of EIO, EROFS, etc. POSIX codes when reading
FS and block manager instance files. The directory manager, in such
cases, labels the data directory in which the bad file resides as failed
in memory. The check that enforces consistency between path instance
metadata files (PIMFs) accounts for such failed directories.
Separately, the introduction of the `kudu fs update_dirs` tool expanded
the logic used to open the FS layout to serve two additional purposes
when running the tool:
- To open the FS layout as the user specified it on a best-effort basis,
ignoring any inconsistencies across PIMFs. This mode allows Kudu to
stage the requested FS layout and test whether existing tablets would
break under the given layout.
- To actually update the FS layout on disk to match the user input.
Walking down the FS-layout-opening path in this mode, Kudu will
create any missing files or directories it encounters along the way.
In this mode, the PIMF consistency check is performed after updating
the appropriate instance files.
The above behavior is currently encapsulated in FsManager::Open() and
the consequent call to DataDirManager::Open().
As mentioned in the JIRA, not accounted for by this previous work, a
disk failure can present itself as a failure to read directory entries
(POSIX code ENOENT), leading to NotFound errors. Therein lies some
conflict in the above logic:
- when opening the FS layout normally (i.e. not running the update tool)
and a NotFound error is encountered (e.g. the user asks for
--fs_data_dirs=/a,/b but only /a can be found), it would be desirable
to treat the affected directory how we currently treat failed
directories
- however, when updating directories, NotFound statuses are still meant
to indicate missing directories to be created.
The questions then become, "How should we treat ENOENT/NotFound errors?
Should they be different than disk failures? If so, how?" There are a
couple options to consider:
1. Lump ENOENT in with the list of POSIX codes that signify a disk
error. This change would mean that semantically, a failed directory
is no different than a missing one, and all of the above mentioned
codepaths must be updated to account for this. Additionally, this
would affect all other codepaths that may yield ENOENT -- codepaths
that previously triggered disk failure handling would trigger that
handling for ENOENT errors.
2. Treat NotFound errors as a special case during update/open only. When
reading FS or block manager instance files, extend disk error
checking to also check for NotFound errors, and treat the errors
similarly, creating in-memory "unhealthy" instances. To open the FS
normally, these unhealthy instances will simply correspond to failed
directories. To update the FS layout, we can leverage the fact that
the FS update tool doesn't support running with any failed
directories, attempting to create all directories and files for _all_
unhealthy instances (both missing and failed) will correctly yield
success in the face of only missing directories and failure in the
presence of any failed directory.
This patch implements Option 2 instead of Option 1. ENOENT is still a
file-specific code, and so lumping it in to trigger DataDir-wide error
handling, even if it may be indicative of disk-wide failure, seems
inappropriate. It could also be argued that if NotFound errors are
discovered at server runtime (e.g. because some malicious user is
deleting files left and right), maybe the best course of action wouldn't
be to treat the directory as failed, but to crash Kudu altogether.
Notes on the new stuff
----------------------
The newly supported scenario is different than starting up Kudu with
extra or missing entries in `fs_data_dirs`, which is still not supported
unless running the update tool.
Examples:
- If an existing server were configured with --fs_data_dirs=/a,/b,/c,
and it were restarted such that only /a,/b existed on disk, Kudu will
start up and list /a,/b,/c, and note that /c is failed.
- If the above server were restarted with --fs_data_dirs=/a,/b, even if
only /a,/b existed on disk, Kudu would fail to start up until running
`kudu fs update_dirs <other flags> --fs_data_dirs=/a,/b`
Some changes in this patch include:
- methods involved in loading PIMFs now treat missing instance files as
"unhealthy", the same way they treat files that fail due disk errors
- DataDirManager::LoadInstances() has been updated to treat missing
PIMFs as "unhealthy", the same way they treat PIMFs that yield disk
errors, returning the successfully loaded, "unhealthy" instances
- a side effect of this is that all user-specified data dirs will have
in-memory DataDirs created for them, even if they don't exist on disk
- various codepaths that previously ended FsManager::Open() with an
IOError/Corruption because all drives were failed will now return
NotFound, indicating Kudu should attempt to create a new FS layout.
This means that a server that has lost all of its data dirs to disk
failures is semantically equivalent to a brand new server; Kudu will
attempt to create a new FS layout in these cases
- as a byproduct of the above changes, when opening the FS layout in
ENFORCE_CONSISTENCY mode with an extra entry in `fs_data_dirs`, Kudu
will fail later than before, at the integrity check, and yield an
IOError instead of a NotFound error
- the UUID and UUID index assignment for missing directories has been
updated when opening the speculative directory manager; see
DataDirManager::Open() for more details.
Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/tools/kudu-tool-test.cc
7 files changed, 269 insertions(+), 143 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/10340/4
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 2:
(4 comments)
Sorry for the tardy response. Not pushing a PS just yet, just responding to a couple points you brought up.
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@62
PS2, Line 62: NotFound, indicating Kudu should attempt to create a new FS layout
> I guess what this also means is that semantically, there's now no differenc
In terms of the control flow of Open/Create/Update paths, somewhat. What differs is the fact that, as implemented, only NotFound() errors will indicate the viability to move forward with UPDATE_ON_DISK paths.
E.g. in FsManager::Open(), when opening instance files, those that fail due to NotFound() errors are separated into a list of missing roots that are created if going down the UPDATE_ON_DISK paths.
This isn't a hard requirement, but I think it's more intuitive, even if it means extra checking for NotFound()s. One alternative you may have alluded to elsewhere would be to treat NotFound() as a disk failure, in which case we might be able to collapse some of the NotFound()-specific codepaths. For an ENFORCE_CONSISTENCY FsManager::Open(), I think this makes perfect sense. I'm not quite sold on doing this for UPDATE_ON_DISK.
We _could_ always try to create missing roots in the face of any disk error codes (including ENOENT); the existing file creation rollback code should make that not too painful. The code changes to make that happen would then revolve around fetching the "failed/missing" (potentially non-canonicalized) roots/instance pathnames from lists that contain some unhealthy root instances or path instances, and using those fetched as the basis for the CreateFileSystemRoots() or CreateNewDataDirectoriesAndUpdateExistingOnes() calls. The downside here is that I think it becomes somewhat less intuitive for readers without knowing the context disk failures and not found errors, although that could be remediated through comments.
Another thing that concerns me wrt lumping ENOENT into the disk error list is that its effects span beyond the scope of startup and fs update. It would affect _any_ missing file, and thus, any data directory with a missing file. Maybe that's ok and desirable, but that call seems like it warrants separate discussion. Or maybe not, I could go either way at this point. WDYT?
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/block_manager_util.cc
File src/kudu/fs/block_manager_util.cc:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/block_manager_util.cc@52
PS2, Line 52: // Evaluates 'status_expr' and if it results in a disk-related error (i.e. disk
> One of the challenging aspects of this change is explaining why this macro
My comment in the commit message addresses this.
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc@603
PS2, Line 603: continue;
> Would it be possible to omit this to simplify the control flow? AFAICT, thi
As implemented, this gives us a clear separation between missing directories, that should be created if we're UPDATING_ON_DISK, and failed directories, that should halt further UPDATE operation. This assumption is used in UpdateExistingInstances(), which doesn't currently handle unhealthy instances, but that can be amended pretty easily.
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc@692
PS2, Line 692: DCHECK(missing_roots.empty());
> If the extent of our recheck is a DCHECK, we could probably omit it and mak
Ack yeah, can do.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 22 May 2018 02:48:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@12
PS2, Line 12: , etc. POSIX codes when reading
: FS and block manager i
> Nit: "reading fs and block manager instance files".
Done
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@21
PS2, Line 21: specifie
> Nit: requested
Done
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/block_manager_util.cc
File src/kudu/fs/block_manager_util.cc:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/block_manager_util.cc@52
PS2, Line 52: // Evaluates 'status_expr' and if it results in a disk-failure error, logs a
> My comment in the commit message addresses this.
Updated this comment with a note, not going into why NotFound errors shouldn't be lumped into IsDiskFailure, but at least explaining why IsNotFound is appropriate here.
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.h@423
PS2, Line 423: It also
: // includes instance files that failed to load because they were missing or
: // because of a disk failure; they are still considered "loaded" and a
> Maybe reword: "It also includes instance files that failed to load because
Done
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc@603
PS2, Line 603: Substitute("could not load $0", instance_filename));
> As implemented, this gives us a clear separation between missing directorie
Done, made LoadInstances() return missing/failed/healthy instances together.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 23 May 2018 20:09:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@54
PS2, Line 54: that previously triggered disk failure handling would trigger that
> Still open.
Done
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@58
PS2, Line 58: d treat the errors
: similarly, creating in-memory "unhealthy" instances. To open
> Nit: singular/plural confusion here
Done
http://gerrit.cloudera.org:8080/#/c/10340/5/src/kudu/fs/block_manager_util.cc
File src/kudu/fs/block_manager_util.cc:
http://gerrit.cloudera.org:8080/#/c/10340/5/src/kudu/fs/block_manager_util.cc@56
PS5, Line 56: PIMFs
> I don't think this acronym is used elsewhere in the code.
Done
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 23 May 2018 22:28:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 4:
Rebased
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 23 May 2018 20:43:27 +0000
Gerrit-HasComments: No
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10340/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/1//COMMIT_MSG@10
PS1, Line 10:
: As a part of previous disk failure work, Kudu currently supports opening
: the FS layout in the face of EIOs when readin
> Just to clarify, I think this patch makes it so that if I pass --fs-data-di
Done
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 19:37:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10340
to look at the new patch set (#3).
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
KUDU-2359: allow startup with missing data dirs
Context
-------
As a part of previous disk failure work, Kudu currently supports opening
the FS layout in the face of EIO, EROFS, etc. POSIX codes when reading
FS and block manager instance files. The directory manager, in such
cases, labels the data directory in which the bad file resides as failed
in memory. The check that enforces consistency between path instance
metadata files (PIMFs) accounts for such failed directories.
Separately, the introduction of the `kudu fs update_dirs` tool expanded
the logic used to open the FS layout to serve two additional purposes
when running the tool:
- To open the FS layout as the user specified it on a best-effort basis,
ignoring any inconsistencies across PIMFs. This mode allows Kudu to
stage the requested FS layout and test whether existing tablets would
break under the given layout.
- To actually update the FS layout on disk to match the user input.
Walking down the FS-layout-opening path in this mode, Kudu will
create any missing files or directories it encounters along the way.
In this mode, the PIMF consistency check is performed after updating
the appropriate instance files.
The above behavior is currently encapsulated in FsManager::Open() and
the consequent call to DataDirManager::Open().
As mentioned in the JIRA, not accounted for by this previous work, a
disk failure can present itself as a failure to read directory entries
(POSIX code ENOENT), leading to NotFound errors. Therein lies some
conflict in the above logic:
- when opening the FS layout normally (i.e. not running the update tool)
and a NotFound error is encountered (e.g. the user asks for
--fs_data_dirs=/a,/b but only /a can be found), it would be desirable
to treat the affected directory how we currently treat failed
directories
- however, when updating directories, NotFound statuses are still meant
to indicate missing directories to be created.
The questions then become, "How should we treat ENOENT/NotFound errors?
Should they be different than disk failures? If so, how?" There are a
couple options to consider:
1. Lump ENOENT in with the list of POSIX codes that signify a disk
error. This change would mean that semantically, a failed directory
is no different than a missing one, and all of the above mentioned
codepaths must be updated to account for this. Additionally, this
would affect all other codepaths that may yield ENOENT -- codepaths
that previously triggered disk failure handling would trigger that
handling for ENOENT errors.
2. Treat NotFound errors as a special case during update/open only. When
reading FS or block manager instance files, extend disk error
checking to also check for NotFound errors, and treat the errors
similarly, creating in-memory "unhealthy" instances. To open the FS
normally, these unhealthy instances will simply correspond to failed
directories. To update the FS layout, we can leverage the fact that
the FS update tool doesn't support running with any failed
directories, attempting to create all directories and files for _all_
unhealthy instances (both missing and failed) will correctly yield
success in the face of only missing directories and failure in the
presence of any failed directory.
This patch implements Option 2 instead of Option 1. ENOENT is still a
file-specific code, and so lumping it in to trigger DataDir-wide error
handling, even if it may be indicative of disk-wide failure, seems
inappropriate. It could also be argued that if NotFound errors are
discovered at server runtime (e.g. because some malicious user is
deleting files left and right), maybe the best course of action wouldn't
be to treat the directory as failed, but to crash Kudu altogether.
Notes on the new stuff
----------------------
The newly supported scenario is different than starting up Kudu with
extra or missing entries in `fs_data_dirs`, which is still not supported
unless running the update tool.
Examples:
- If an existing server were configured with --fs_data_dirs=/a,/b,/c,
and it were restarted such that only /a,/b existed on disk, Kudu will
start up and list /a,/b,/c, and note that /c is failed.
- If the above server were restarted with --fs_data_dirs=/a,/b, even if
only /a,/b existed on disk, Kudu would fail to start up until running
`kudu fs update_dirs <other flags> --fs_data_dirs=/a,/b`
Some changes in this patch include:
- methods involved in loading PIMFs now treat missing instance files as
"unhealthy", the same way they treat files that fail due disk errors
- DataDirManager::LoadInstances() has been updated to treat missing
PIMFs as "unhealthy", the same way they treat PIMFs that yield disk
errors, returning the successfully loaded, "unhealthy" instances
- a side effect of this is that all user-specified data dirs will have
in-memory DataDirs created for them, even if they don't exist on disk
- various codepaths that previously ended FsManager::Open() with an
IOError/Corruption because all drives were failed will now return
NotFound, indicating Kudu should attempt to create a new FS layout.
This means that a server that has lost all of its data dirs to disk
failures is semantically equivalent to a brand new server; Kudu will
attempt to create a new FS layout in these cases
- as a byproduct of the above changes, when opening the FS layout in
ENFORCE_CONSISTENCY mode with an extra entry in `fs_data_dirs`, Kudu
will fail later than before, at the integrity check, and yield an
IOError instead of a NotFound error
- the UUID and UUID index assignment for missing directories has been
updated when opening the speculative directory manager; see
DataDirManager::Open() for more details.
Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/tools/kudu-tool-test.cc
7 files changed, 272 insertions(+), 143 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/10340/3
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 2:
> Patch Set 1:
>
> (1 comment)
Yep, that's all correct. I still think this commit message could be improved, but for now I've at least added a bit more context to frame the issue better.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 19:37:00 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
KUDU-2359: allow startup with missing data dirs
Context
-------
As a part of previous disk failure work, Kudu currently supports opening
the FS layout in the face of EIO, EROFS, etc. POSIX codes when reading
FS and block manager instance files. The directory manager, in such
cases, labels the data directory in which the bad file resides as failed
in memory. The check that enforces consistency between path instance
metadata files (PIMFs) accounts for such failed directories.
Separately, the introduction of the `kudu fs update_dirs` tool expanded
the logic used to open the FS layout to serve two additional purposes
when running the tool:
- To open the FS layout as the user specified it on a best-effort basis,
ignoring any inconsistencies across PIMFs. This mode allows Kudu to
stage the requested FS layout and test whether existing tablets would
break under the given layout.
- To actually update the FS layout on disk to match the user input.
Walking down the FS-layout-opening path in this mode, Kudu will
create any missing files or directories it encounters along the way.
In this mode, the PIMF consistency check is performed after updating
the appropriate instance files.
The above behavior is currently encapsulated in FsManager::Open() and
the consequent call to DataDirManager::Open().
As mentioned in the JIRA, not accounted for by this previous work, a
disk failure can present itself as a failure to read directory entries
(POSIX code ENOENT), leading to NotFound errors. Therein lies some
conflict in the above logic:
- when opening the FS layout normally (i.e. not running the update tool)
and a NotFound error is encountered (e.g. the user asks for
--fs_data_dirs=/a,/b but only /a can be found), it would be desirable
to treat the affected directory how we currently treat failed
directories
- however, when updating directories, NotFound statuses are still meant
to indicate missing directories to be created.
The questions then become, "How should we treat ENOENT/NotFound errors?
Should they be different than disk failures? If so, how?" There are a
couple options to consider:
1. Lump ENOENT in with the list of POSIX codes that signify a disk
error. This change would mean that semantically, a failed directory
is no different than a missing one, and all of the above mentioned
codepaths must be updated to account for this. Additionally, this
would affect all other codepaths that may yield ENOENT -- codepaths
that previously triggered disk failure handling would trigger that
handling for ENOENT errors.
2. Treat NotFound errors as a special case during update/open only. When
reading FS or block manager instance files, extend disk error
checking to also check for NotFound errors, and treat the errors
similarly, creating in-memory "unhealthy" instances. To open the FS
normally, these unhealthy instances will simply correspond to failed
directories. To update the FS layout, we can leverage the fact that
the FS update tool doesn't support running with any failed
directories, attempting to create all directories and files for _all_
unhealthy instances (both missing and failed) will correctly yield
success in the face of only missing directories and failure in the
presence of any failed directory.
This patch implements Option 2 instead of Option 1. ENOENT is still a
file-specific code, and so lumping it in to trigger DataDir-wide error
handling, even if it may be indicative of disk-wide failure, seems
inappropriate. It could also be argued that if NotFound errors are
discovered at server runtime (e.g. because some malicious user is
deleting files left and right), maybe the best course of action wouldn't
be to treat the directory as failed, but to crash Kudu altogether.
Notes on the new stuff
----------------------
The newly supported scenario is different than starting up Kudu with
extra or missing entries in `fs_data_dirs`, which is still not supported
unless running the update tool.
Examples:
- If an existing server were configured with --fs_data_dirs=/a,/b,/c,
and it were restarted such that only /a,/b existed on disk, Kudu will
start up and list /a,/b,/c, and note that /c is failed.
- If the above server were restarted with --fs_data_dirs=/a,/b, even if
only /a,/b existed on disk, Kudu would fail to start up until running
`kudu fs update_dirs <other flags> --fs_data_dirs=/a,/b`
Some changes in this patch include:
- methods involved in loading PIMFs now treat missing instance files as
"unhealthy", the same way they treat files that fail due to disk
errors
- DataDirManager::LoadInstances() has been updated to treat missing
PIMFs as "unhealthy", the same way they treat PIMFs that yield disk
errors, returning the successfully loaded, "unhealthy" instances
- a side effect of this is that all user-specified data dirs will have
in-memory DataDirs created for them, even if they don't exist on disk
- various codepaths that previously ended FsManager::Open() with an
IOError/Corruption because all drives were failed will now return
NotFound, indicating Kudu should attempt to create a new FS layout.
This means that a server that has lost all of its data dirs to disk
failures is semantically equivalent to a brand new server; Kudu will
attempt to create a new FS layout in these cases
- as a byproduct of the above changes, when opening the FS layout in
ENFORCE_CONSISTENCY mode with an extra entry in `fs_data_dirs`, Kudu
will fail later than before, at the integrity check, and yield an
IOError instead of a NotFound error
- the UUID and UUID index assignment for missing directories has been
updated when opening the speculative directory manager; see
DataDirManager::Open() for more details.
Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Reviewed-on: http://gerrit.cloudera.org:8080/10340
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/tools/kudu-tool-test.cc
7 files changed, 268 insertions(+), 143 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
Todd Lipcon: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 6: Code-Review+1
I've kinda lost context on the code-level details of this, but reading through the commit message it sounds like the approach makes sense.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 23 May 2018 23:04:04 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10340
to look at the new patch set (#5).
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
KUDU-2359: allow startup with missing data dirs
Context
-------
As a part of previous disk failure work, Kudu currently supports opening
the FS layout in the face of EIO, EROFS, etc. POSIX codes when reading
FS and block manager instance files. The directory manager, in such
cases, labels the data directory in which the bad file resides as failed
in memory. The check that enforces consistency between path instance
metadata files (PIMFs) accounts for such failed directories.
Separately, the introduction of the `kudu fs update_dirs` tool expanded
the logic used to open the FS layout to serve two additional purposes
when running the tool:
- To open the FS layout as the user specified it on a best-effort basis,
ignoring any inconsistencies across PIMFs. This mode allows Kudu to
stage the requested FS layout and test whether existing tablets would
break under the given layout.
- To actually update the FS layout on disk to match the user input.
Walking down the FS-layout-opening path in this mode, Kudu will
create any missing files or directories it encounters along the way.
In this mode, the PIMF consistency check is performed after updating
the appropriate instance files.
The above behavior is currently encapsulated in FsManager::Open() and
the consequent call to DataDirManager::Open().
As mentioned in the JIRA, not accounted for by this previous work, a
disk failure can present itself as a failure to read directory entries
(POSIX code ENOENT), leading to NotFound errors. Therein lies some
conflict in the above logic:
- when opening the FS layout normally (i.e. not running the update tool)
and a NotFound error is encountered (e.g. the user asks for
--fs_data_dirs=/a,/b but only /a can be found), it would be desirable
to treat the affected directory how we currently treat failed
directories
- however, when updating directories, NotFound statuses are still meant
to indicate missing directories to be created.
The questions then become, "How should we treat ENOENT/NotFound errors?
Should they be different than disk failures? If so, how?" There are a
couple options to consider:
1. Lump ENOENT in with the list of POSIX codes that signify a disk
error. This change would mean that semantically, a failed directory
is no different than a missing one, and all of the above mentioned
codepaths must be updated to account for this. Additionally, this
would affect all other codepaths that may yield ENOENT -- codepaths
that previously triggered disk failure handling would trigger that
handling for ENOENT errors.
2. Treat NotFound errors as a special case during update/open only. When
reading FS or block manager instance files, extend disk error
checking to also check for NotFound errors, and treat the errors
similarly, creating in-memory "unhealthy" instances. To open the FS
normally, these unhealthy instances will simply correspond to failed
directories. To update the FS layout, we can leverage the fact that
the FS update tool doesn't support running with any failed
directories, attempting to create all directories and files for _all_
unhealthy instances (both missing and failed) will correctly yield
success in the face of only missing directories and failure in the
presence of any failed directory.
This patch implements Option 2 instead of Option 1. ENOENT is still a
file-specific code, and so lumping it in to trigger DataDir-wide error
handling, even if it may be indicative of disk-wide failure, seems
inappropriate. It could also be argued that if NotFound errors are
discovered at server runtime (e.g. because some malicious user is
deleting files left and right), maybe the best course of action wouldn't
be to treat the directory as failed, but to crash Kudu altogether.
Notes on the new stuff
----------------------
The newly supported scenario is different than starting up Kudu with
extra or missing entries in `fs_data_dirs`, which is still not supported
unless running the update tool.
Examples:
- If an existing server were configured with --fs_data_dirs=/a,/b,/c,
and it were restarted such that only /a,/b existed on disk, Kudu will
start up and list /a,/b,/c, and note that /c is failed.
- If the above server were restarted with --fs_data_dirs=/a,/b, even if
only /a,/b existed on disk, Kudu would fail to start up until running
`kudu fs update_dirs <other flags> --fs_data_dirs=/a,/b`
Some changes in this patch include:
- methods involved in loading PIMFs now treat missing instance files as
"unhealthy", the same way they treat files that fail due disk errors
- DataDirManager::LoadInstances() has been updated to treat missing
PIMFs as "unhealthy", the same way they treat PIMFs that yield disk
errors, returning the successfully loaded, "unhealthy" instances
- a side effect of this is that all user-specified data dirs will have
in-memory DataDirs created for them, even if they don't exist on disk
- various codepaths that previously ended FsManager::Open() with an
IOError/Corruption because all drives were failed will now return
NotFound, indicating Kudu should attempt to create a new FS layout.
This means that a server that has lost all of its data dirs to disk
failures is semantically equivalent to a brand new server; Kudu will
attempt to create a new FS layout in these cases
- as a byproduct of the above changes, when opening the FS layout in
ENFORCE_CONSISTENCY mode with an extra entry in `fs_data_dirs`, Kudu
will fail later than before, at the integrity check, and yield an
IOError instead of a NotFound error
- the UUID and UUID index assignment for missing directories has been
updated when opening the speculative directory manager; see
DataDirManager::Open() for more details.
Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/tools/kudu-tool-test.cc
7 files changed, 268 insertions(+), 143 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/10340/5
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10340
to look at the new patch set (#6).
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
KUDU-2359: allow startup with missing data dirs
Context
-------
As a part of previous disk failure work, Kudu currently supports opening
the FS layout in the face of EIO, EROFS, etc. POSIX codes when reading
FS and block manager instance files. The directory manager, in such
cases, labels the data directory in which the bad file resides as failed
in memory. The check that enforces consistency between path instance
metadata files (PIMFs) accounts for such failed directories.
Separately, the introduction of the `kudu fs update_dirs` tool expanded
the logic used to open the FS layout to serve two additional purposes
when running the tool:
- To open the FS layout as the user specified it on a best-effort basis,
ignoring any inconsistencies across PIMFs. This mode allows Kudu to
stage the requested FS layout and test whether existing tablets would
break under the given layout.
- To actually update the FS layout on disk to match the user input.
Walking down the FS-layout-opening path in this mode, Kudu will
create any missing files or directories it encounters along the way.
In this mode, the PIMF consistency check is performed after updating
the appropriate instance files.
The above behavior is currently encapsulated in FsManager::Open() and
the consequent call to DataDirManager::Open().
As mentioned in the JIRA, not accounted for by this previous work, a
disk failure can present itself as a failure to read directory entries
(POSIX code ENOENT), leading to NotFound errors. Therein lies some
conflict in the above logic:
- when opening the FS layout normally (i.e. not running the update tool)
and a NotFound error is encountered (e.g. the user asks for
--fs_data_dirs=/a,/b but only /a can be found), it would be desirable
to treat the affected directory how we currently treat failed
directories
- however, when updating directories, NotFound statuses are still meant
to indicate missing directories to be created.
The questions then become, "How should we treat ENOENT/NotFound errors?
Should they be different than disk failures? If so, how?" There are a
couple options to consider:
1. Lump ENOENT in with the list of POSIX codes that signify a disk
error. This change would mean that semantically, a failed directory
is no different than a missing one, and all of the above mentioned
codepaths must be updated to account for this. Additionally, this
would affect all other codepaths that may yield ENOENT -- codepaths
that previously triggered disk failure handling would trigger that
handling for ENOENT errors.
2. Treat NotFound errors as a special case during update/open only. When
reading FS or block manager instance files, extend disk error
checking to also check for NotFound errors, and treat the errors
similarly, creating in-memory "unhealthy" instances. To open the FS
normally, these unhealthy instances will simply correspond to failed
directories. To update the FS layout, we can leverage the fact that
the FS update tool doesn't support running with any failed
directories, attempting to create all directories and files for _all_
unhealthy instances (both missing and failed) will correctly yield
success in the face of only missing directories and failure in the
presence of any failed directory.
This patch implements Option 2 instead of Option 1. ENOENT is still a
file-specific code, and so lumping it in to trigger DataDir-wide error
handling, even if it may be indicative of disk-wide failure, seems
inappropriate. It could also be argued that if NotFound errors are
discovered at server runtime (e.g. because some malicious user is
deleting files left and right), maybe the best course of action wouldn't
be to treat the directory as failed, but to crash Kudu altogether.
Notes on the new stuff
----------------------
The newly supported scenario is different than starting up Kudu with
extra or missing entries in `fs_data_dirs`, which is still not supported
unless running the update tool.
Examples:
- If an existing server were configured with --fs_data_dirs=/a,/b,/c,
and it were restarted such that only /a,/b existed on disk, Kudu will
start up and list /a,/b,/c, and note that /c is failed.
- If the above server were restarted with --fs_data_dirs=/a,/b, even if
only /a,/b existed on disk, Kudu would fail to start up until running
`kudu fs update_dirs <other flags> --fs_data_dirs=/a,/b`
Some changes in this patch include:
- methods involved in loading PIMFs now treat missing instance files as
"unhealthy", the same way they treat files that fail due to disk
errors
- DataDirManager::LoadInstances() has been updated to treat missing
PIMFs as "unhealthy", the same way they treat PIMFs that yield disk
errors, returning the successfully loaded, "unhealthy" instances
- a side effect of this is that all user-specified data dirs will have
in-memory DataDirs created for them, even if they don't exist on disk
- various codepaths that previously ended FsManager::Open() with an
IOError/Corruption because all drives were failed will now return
NotFound, indicating Kudu should attempt to create a new FS layout.
This means that a server that has lost all of its data dirs to disk
failures is semantically equivalent to a brand new server; Kudu will
attempt to create a new FS layout in these cases
- as a byproduct of the above changes, when opening the FS layout in
ENFORCE_CONSISTENCY mode with an extra entry in `fs_data_dirs`, Kudu
will fail later than before, at the integrity check, and yield an
IOError instead of a NotFound error
- the UUID and UUID index assignment for missing directories has been
updated when opening the speculative directory manager; see
DataDirManager::Open() for more details.
Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/tools/kudu-tool-test.cc
7 files changed, 268 insertions(+), 143 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/10340/6
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 2:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@12
PS2, Line 12: reading instance and block
: manager instance files
Nit: "reading fs and block manager instance files".
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@21
PS2, Line 21: requests
Nit: requested
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@35
PS2, Line 35: A missing data
: directory in this case is one that has no instance or block manager
: instance file
Would it be useful to broaden the scope to include missing data_root directories or data_dir subdirectories as well?
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@54
PS2, Line 54: "unhealthy", the same way they treat files that fail due disk errors
Nit: due to
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@58
PS2, Line 58: missing directories as
: an unhealthy instance used to spawn a failed DataDir in memory
Nit: singular/plural confusion here
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@62
PS2, Line 62: NotFound, indicating Kudu should attempt to create a new FS layout
I guess what this also means is that semantically, there's now no difference between "brand new node" and "node whose disks all failed because instance files could not be found". Was that important before?
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/block_manager_util.cc
File src/kudu/fs/block_manager_util.cc:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/block_manager_util.cc@52
PS2, Line 52: // Evaluates 'status_expr' and if it results in a disk-related error (i.e. disk
One of the challenging aspects of this change is explaining why this macro should consider NotFound errors as disk failures, but why we don't want to modify IsDiskFailure outright to include NotFound.
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.h@423
PS2, Line 423: Instance
: // files that fail to load because they are missing or because of a disk
: // failure are considered loaded and are internally labeled unhealthy.
Maybe reword: "It also includes instance files that failed to load because they were missing or because of a disk failure; they are still considered "loaded" and are labeled unhealthy internally".
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc@603
PS2, Line 603: continue;
Would it be possible to omit this to simplify the control flow? AFAICT, this would mean loaded_instances always has missing (and unhealthy) instances in it, even if missing_roots was set. Is that a big deal?
http://gerrit.cloudera.org:8080/#/c/10340/2/src/kudu/fs/data_dirs.cc@692
PS2, Line 692: DCHECK(missing_roots.empty());
If the extent of our recheck is a DCHECK, we could probably omit it and make L696 apply to both code paths, no?
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 11 May 2018 21:23:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10340/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/1//COMMIT_MSG@10
PS1, Line 10: Note that this behavior is different than starting
: up Kudu with extra data directories in `fs_data_dirs`, which is still
: not supported unless running the update tool.
Just to clarify, I think this patch makes it so that if I pass --fs-data-dirs=/a,/b,/c and it turns out /c is missing on disk, the server still starts up. However, if I pass --fs-data-dirs=/a,/b, the server fails to start, regardless of whether /c is missing or not. That persists until I run 'kudu fs update_data_dirs --fs-data-dirs=/a,/b".
Is all this correct? If so, could you clarify this part of the commit message to say that this behavior is different than starting up Kudu with extra or with missing entries in `fs_data_dirs`? The first time I read this I thought that by not mentioning "missing entries", the patch was allowing for that kind of "allow startup with missing data dirs" too, which, as far as I can tell, isn't what KUDU-2359 was requesting.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 18:33:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10340
to look at the new patch set (#2).
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
WIP KUDU-2359: allow startup with missing data dirs
Context
-------
As a part of previous disk failure work, Kudu currently supports opening
the FS layout in the face of EIOs when reading instance and block
manager instance files. The directory manager, in such cases, labels the
data directory in which the bad file resides as failed in memory. The
check that enforces consistency between instance files accounts for such
failed DataDirs.
Separately, the introduction of the `kudu fs update_dirs` tool expanded
the logic used to open the FS layout to serve two new purposes when
running the tool:
- To open the FS layout as the user requests it, ignoring any
inconsistencies across data directories. This mode allows Kudu to
stage the requested FS layout and test whether existing tablets would
break due to the update. In this mode, the consistency check is
skipped entirely.
- To actually update the FS layout on disk to match the user input. In
this mode, the consistency check is performed after updating the
appropriate instance files.
New stuff
---------
As mentioned in the JIRA, disk failures can manifest as a failure to
read directory entries, leading to NotFound errors. As such, this patch
reconciles the above features while making the adjustments necessary to
support Kudu starting up with missing data directories. A missing data
directory in this case is one that has no instance or block manager
instance file; upon startup, Kudu will treat such data dirs as failed.
Note that this behavior is different than starting up Kudu with extra or
missing entries in `fs_data_dirs`, which is still not supported unless
running the update tool.
Examples:
- If an existing server were configured with --fs_data_dirs=/a,/b,/c,
and it were restarted such that only /a,/b existed on disk, Kudu will
start up and list /a,/b,/c, and note that /c is failed.
- If the above server were restarted with --fs_data_dirs=/a,/b, even if
only /a,/b existed on disk, Kudu would fail to start up until running
`kudu fs update_dirs [other flags] --fs_data_dirs=/a,/b`
Some notes and changes in this patch include:
- methods involved in loading block manager instance files
(PathInstanceMetadataFiles) now treat missing instance files as
"unhealthy", the same way they treat files that fail due disk errors
- DataDirManager::LoadInstances() has been updated to handle the above
change, optionally returning the missing instances as unhealthy
instances. This allows the update codepaths to explicitly track missing
directories and the startup codepaths to treat missing directories as
an unhealthy instance used to spawn a failed DataDir in memory
- various codepaths that previously ended FsManager::Open() with an
IOError/Corruption because all drives were failed will now return
NotFound, indicating Kudu should attempt to create a new FS layout
- as a byproduct of the above changes, when opening the FS layout in
ENFORCE_CONSISTENCY mode with an extra data dir included in
`fs_data_dirs`, Kudu will fail later than before, at the integrity
check, and yield an IOError instead of a NotFound error
- the UUID and UUID index assignment for missing/failed directories has
been updated when opening the speculative directory manager; see
DataDirManager::Open() for more details.
WIP this change touches codepaths with a fair number of edge cases, and
as such, I haven't fully convinced myself this is the best approach.
That said, it is reviewable and has tests passing.
Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
---
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/data_dirs-test.cc
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/tools/kudu-tool-test.cc
7 files changed, 259 insertions(+), 126 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/10340/2
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2359: allow startup with missing data dirs
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 6: Code-Review+2
Looks good, though wondering if you want to collect a review from Todd too since this is somewhat complex and he was previously involved in KUDU-2359.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 23 May 2018 22:32:55 +0000
Gerrit-HasComments: No
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10340/2//COMMIT_MSG@62
PS2, Line 62: NotFound, indicating Kudu should attempt to create a new FS layout
> In terms of the control flow of Open/Create/Update paths, somewhat. What di
Agreed with the feeling that treating ENOENT as a disk error would require some more thinking. I'd only feel comfortable doing it if we audited the block managers to understand all the places where we might see ENOENT.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 22 May 2018 20:54:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2359: allow startup with missing data dirs
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10340 )
Change subject: WIP KUDU-2359: allow startup with missing data dirs
......................................................................
Patch Set 1:
I'm going to add to/rewrite this commit message to add more context surrounding disk failure handling and the update tool.
--
To view, visit http://gerrit.cloudera.org:8080/10340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I61a71265c3cc34a7b72320149770a814ec7f8351
Gerrit-Change-Number: 10340
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 08 May 2018 17:34:03 +0000
Gerrit-HasComments: No