subject:"\[kudu\-CR\] KUDU\-2202\: support for removing data directories \(take two\)"

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-21 Thread Adar Dembo (Code Review)

Adar Dembo has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. The DataDirManager and FsManager's update_on_disk boolean option has been
   converted into a tri-state. The new IGNORE_INCONSISTENCY state is used by
   the CLI tool to create a "speculative" FsManager using the new data dir
   configuration. This FsManager is used to check whether existing tablets
   depend on the data dir to be removed without making destructive changes
   to on-disk data structures.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Reviewed-on: http://gerrit.cloudera.org:8080/8978
Reviewed-by: Andrew Wong 
Tested-by: Kudu Jenkins
---
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
7 files changed, 441 insertions(+), 92 deletions(-)

Approvals:
  Andrew Wong: Looks good to me, approved
  Kudu Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 11
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-21 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 10: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 10
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 22 Jan 2018 03:52:38 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-21 Thread Adar Dembo (Code Review)

Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8978

to look at the new patch set (#10).

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. The DataDirManager and FsManager's update_on_disk boolean option has been
   converted into a tri-state. The new IGNORE_INCONSISTENCY state is used by
   the CLI tool to create a "speculative" FsManager using the new data dir
   configuration. This FsManager is used to check whether existing tablets
   depend on the data dir to be removed without making destructive changes
   to on-disk data structures.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
7 files changed, 441 insertions(+), 92 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/10
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 10
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-21 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc@699
PS8, Line 699:
 :
 :   // All instances are present and accounted for. Time to create 
the in-memory
 :   // data directory structures.
> Sorry I wasn't explicit, I meant moving this outside of the if block and re
Oh, I understand what you mean now. Yes, I agree this makes more sense. Done.



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 10
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 22 Jan 2018 03:44:50 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-21 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 9: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc@699
PS8, Line 699: RETURN_NOT_OK_PREPEND(
 : 
PathInstanceMetadataFile::CheckIntegrity(loaded_instances),
 : Substitute("could not verify integrity of files after 
data directory update: $0",
 :JoinStrings(GetDataDirs(), ",")));
> Yeah the added encapsulation isn't a bad thing, but bear in mind that we'll
Sorry I wasn't explicit, I meant moving this outside of the if block and 
removing the check at L651.



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 9
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 22 Jan 2018 01:16:14 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-21 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc@699
PS8, Line 699: RETURN_NOT_OK_PREPEND(
 : 
PathInstanceMetadataFile::CheckIntegrity(loaded_instances),
 : Substitute("could not verify integrity of files after 
data directory update: $0",
 :JoinStrings(GetDataDirs(), ",")));
> nit: could merge this CheckIntegrity with the above with:
Yeah the added encapsulation isn't a bad thing, but bear in mind that we'll 
only end up down here if we're in UPDATE_ON_DISK mode (L659), so the 
consistency_check condition is guaranteed to be true.


http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc@812
PS8, Line 812: IGNORE_CONSISTENCY
> nit: IGNORE_INCONSISTENCY
Done



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sun, 21 Jan 2018 20:14:51 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-21 Thread Adar Dembo (Code Review)

Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8978

to look at the new patch set (#9).

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. The DataDirManager and FsManager's update_on_disk boolean option has been
   converted into a tri-state. The new IGNORE_INCONSISTENCY state is used by
   the CLI tool to create a "speculative" FsManager using the new data dir
   configuration. This FsManager is used to check whether existing tablets
   depend on the data dir to be removed without making destructive changes
   to on-disk data structures.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
7 files changed, 443 insertions(+), 90 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/9
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 9
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-19 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 8: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc@699
PS8, Line 699: RETURN_NOT_OK_PREPEND(
 : 
PathInstanceMetadataFile::CheckIntegrity(loaded_instances),
 : Substitute("could not verify integrity of files after 
data directory update: $0",
 :JoinStrings(GetDataDirs(), ",")));
nit: could merge this CheckIntegrity with the above with:

 if (opts_.consistency_check != ConsistencyCheckBehavior::IGNORE_INCONSISTENCY) 
{
   RETURN_NOT_OK(CheckIntegrity());
 }

This seems more explicitly aligned with the ConsistencyCheckBehavior enums (ie 
if we're ignoring inconsistencies, there's no need to check integrity). I guess 
with the current approach you can distinguish the error messages. Not a 
critical change at all, so up to you.


http://gerrit.cloudera.org:8080/#/c/8978/8/src/kudu/fs/data_dirs.cc@812
PS8, Line 812: IGNORE_CONSISTENCY
nit: IGNORE_INCONSISTENCY



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 20 Jan 2018 01:13:59 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-19 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 8: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 20 Jan 2018 00:49:18 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-19 Thread Adar Dembo (Code Review)

Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8978

to look at the new patch set (#8).

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. The DataDirManager and FsManager's update_on_disk boolean option has been
   converted into a tri-state. The new IGNORE_INCONSISTENCY state is used by
   the CLI tool to create a "speculative" FsManager using the new data dir
   configuration. This FsManager is used to check whether existing tablets
   depend on the data dir to be removed without making destructive changes
   to on-disk data structures.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
7 files changed, 443 insertions(+), 90 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/8
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-19 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.h@104
PS7, Line 104:   // If 'update_on_disk' and 'read_only' are both true, the 
FsManager allows
 :   // 'data_roots' to deviate from their on-disk data structures 
without
 :   // updating the latter to match the former.
> I'm not really a fan of these semantics -- it seems somewhat confusing to b
I had been thinking of just renaming update_on_disk to something like 
ignore_inconsistencies (and doc that it may update the on-disk structures), but 
I like your suggestion more. Done.


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc@347
PS7, Line 347:   if (!missing_roots.empty() && !opts_.read_only) {
> missing_roots will only be non-empty if opts.update_on_disk is true (as per
Yeah, I had added a DCHECK and comment to that effect to the equivalent change 
in data_dirs.cc, but not here:

  // Add new or remove existing data directories, if desired.
  if (!opts_.read_only &&
  (!missing_roots.empty() || has_missing_instance)) {
DCHECK(opts_.update_on_disk); // guaranteed by LoadInstances and
  // the ternary in CheckIntegrity above

Anyway, we don't need the read_only check any more because read_only + 
update_on_disk is forbidden once again. I'll update this.


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/kudu-tool-test.cc@2251
PS7, Line 2251:   // second table should have all failed.
> can we add one more step in this test which re-adds the force-removed direc
It should work, but the current code fails in the integrity checks when 
re-adding a previously removed data directory.

So I modified DataDirManager::Open to elide all integrity checks when updating. 
This also allowed me to get rid of the CheckIntegrity changes which were pretty 
ugly to begin with. I also beefed up this test and added a couple more 
fs_manager-test cases.

Note that in real life, although the tserver can unfail these tablets, the 
master may kick off rereplication for them anyway.


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/tool_action_fs.cc@330
PS7, Line 330:   // If the user specifies --force, we assume they know what 
they're doing and
 :   // skip this check.
> would it be possible to do the check in a new function, and something like:
Done



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 20 Jan 2018 00:12:41 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-18 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 7:

(1 comment)

With the metadata dir patch in, it's probably worth at least discussing (and 
maybe testing) the ways in which affects the remove tool's behavior. I don't 
think much changes, but something like:

- Metadata dirs in WAL dir: removing any data dir is fine. No configuration 
changes necessary.
- Metadata dirs in the first data dir: This is also fine, but further 
deployments must now specify the metadata dir, as it is no longer referenced in 
fs_data_dirs.
- Metadata dirs in any other directory: same as the WAL case.

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc@347
PS7, Line 347:   if (!missing_roots.empty() && !opts_.read_only) {
> huh, is this an existing bug? if you use an fs_manager tool and pass an ext
missing_roots will only be non-empty if opts.update_on_disk is true (as per 
L292). This should probably be in a comment.



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 22:44:49 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-18 Thread Todd Lipcon (Code Review)

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.h@104
PS7, Line 104:   // If 'update_on_disk' and 'read_only' are both true, the 
FsManager allows
 :   // 'data_roots' to deviate from their on-disk data structures 
without
 :   // updating the latter to match the former.
I'm not really a fan of these semantics -- it seems somewhat confusing to both 
ask to "update" and also to "not write". I prefer read_only to just enforce a 
sort of "fail any write ops" rather than in some cases also meaning "skip the 
write op."

Would it be feasible to change this to some kind of enum like:

enum ConsistencyCheckBehavior {
  // If data_roots doesn't match the on-disk path sets, fail.
  ENFORCE_CONSISTENCY,

  // If data_roots doesn't match the on-disk path sets, update the
  // on-disk data to match. Requires 'read_only' to be false.
  UPDATE_ON_DISK,

  // If data_roots doesn't match the on-disk path sets,
  // continue without updating the on-disk data.
  IGNORE_INCONSISTENCY
};


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/fs/fs_manager.cc@347
PS7, Line 347:   if (!missing_roots.empty() && !opts_.read_only) {
huh, is this an existing bug? if you use an fs_manager tool and pass an extra 
dir, it gets created?


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/kudu-tool-test.cc@2251
PS7, Line 2251:   // second table should have all failed.
can we add one more step in this test which re-adds the force-removed 
directory? Would that properly re-enable the missing tablets? Seems like a 
useful "oops!" recovery option for an admin who runs the wrong command.


http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/7/src/kudu/tools/tool_action_fs.cc@330
PS7, Line 330:   // If the user specifies --force, we assume they know what 
they're doing and
 :   // skip this check.
would it be possible to do the check in a new function, and something like:

Status check_result = CheckTabletsWontFailWithUpdate(...);
if (FLAGS_force) {
   WARN_NOT_OK(check_result, "blah blah continuing because you said force");
} else {
  RETURN_NOT_OK_PREPEND(check_result, "blah blah");
}

This way if someone uses --force and then pastes us the logs of what they did, 
we'll at least see what happened



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 18 Jan 2018 19:35:47 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-17 Thread Adar Dembo (Code Review)

Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8978

to look at the new patch set (#7).

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. PathInstanceMetadataFile::CheckIntegrity can now be run without checking
   for missing instance files. This is used when removing a data dir (its
   instance file will be missing).
2. The DataDirManager and FsManager may now be constructed with both
   read_only=true and update_on_disk=true. The CLI tool leverages this
   functionality to create a "speculative" FsManager using the new data dir
   configuration that is used to check whether existing tablets depend on
   the data dir to be removed.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
10 files changed, 344 insertions(+), 90 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/7
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 7
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-16 Thread Adar Dembo (Code Review)

Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8978

to look at the new patch set (#5).

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. PathInstanceMetadataFile::CheckIntegrity can now be run without checking
   for missing instance files. This is used when removing a data dir (its
   instance file will be missing).
2. The DataDirManager and FsManager may now be constructed with both
   read_only=true and update_on_disk=true. The CLI tool leverages this
   functionality to create a "speculative" FsManager using the new data dir
   configuration that is used to check whether existing tablets depend on
   the data dir to be removed.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
10 files changed, 344 insertions(+), 90 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/5
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-16 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8978/4/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/4/src/kudu/fs/data_dirs.cc@810
PS4, Line 810: list of data directories now
 : // includes the missing directory.
> nit: If I'm understanding this correctly, the update should have removed th
Ah yes, will update.



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 16 Jan 2018 19:17:50 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-12 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8978/4/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/4/src/kudu/fs/data_dirs.cc@810
PS4, Line 810: list of data directories now
 : // includes the missing directory.
nit: If I'm understanding this correctly, the update should have removed the 
UUID, so maybe replace "includes" with "excludes"?



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Sat, 13 Jan 2018 03:42:45 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-10 Thread Adar Dembo (Code Review)

Hello Kudu Jenkins, Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8978

to look at the new patch set (#4).

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. PathInstanceMetadataFile::CheckIntegrity can now be run without checking
   for missing instance files. This is used when removing a data dir (its
   instance file will be missing).
2. The DataDirManager and FsManager may now be constructed with both
   read_only=true and update_on_disk=true. The CLI tool leverages this
   functionality to create a "speculative" FsManager using the new data dir
   configuration that is used to check whether existing tablets depend on
   the data dir to be removed.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
10 files changed, 343 insertions(+), 90 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/4
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-10 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:

http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.h@222
PS3, Line 222: without updating the latter
> nit: the naming here gives me pause since the name is no longer accurate, b
Yeah, I couldn't think of a better name either.


http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.cc@810
PS3, Line 810:  the missing directory made its way
 : // into the list of data directories.
> nit: at first glance, I read this as, "We have a missing directory and it i
Done



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jan 2018 03:47:40 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-10 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 3:

(5 comments)

Just a couple more nits from me

http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:

http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.h@222
PS3, Line 222: without updating the latter
nit: the naming here gives me pause since the name is no longer accurate, but I 
can't settle on a more fitting name. Maybe update_instances? 
update_directories? Though maybe this comment is clear enough to not warrant 
any changes.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@805
PS1, Line 805: // happen when opts_.read_only and opts_.update_on_disk are true 
(i.e.
 : // if this DataDirManager is trialing a new data dir 
confi
> OK, I took another stab at this comment. Let me know what you think.
Much clearer, thanks!


http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/3/src/kudu/fs/data_dirs.cc@810
PS3, Line 810:  the missing directory made its way
 : // into the list of data directories.
nit: at first glance, I read this as, "We have a missing directory and it is 
added to the list of data directories". Maybe flip it for clarity: "...the list 
of [available uuids?] accounts for the missing directory".


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc@2232
PS1, Line 2232: // Make sure the failure reall
> We could, but is it necessary? We wouldn't expect tablets to just disappear
Ack


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc@854
PS1, Line 854: "Updates the set of data directories in an existing Kudu 
filesystem")
 :   .ExtraDescription("If a data directory is in use by a 
tablet and is "
 :   "removed, the operation will fail unless --force is 
also used")
 :   .AddOptionalParameter("fs_wal_dir")
 :   .AddOptionalParameter("fs_data_dirs")
 :   .AddOptionalParameter("force", boost::none, string("If 
true, permits "
 :   "the removal of a data directory that is configured 
for use by "
 :   "existing tablets. Those tablets will fail the next 
time the server "
 :   "is started"))
> I'm not sure; as you can see, List uses 'tablets' instead of 'local replica
Ack



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jan 2018 00:58:45 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-10 Thread Adar Dembo (Code Review)

Hello Andrew Wong, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8978

to look at the new patch set (#3).

Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. PathInstanceMetadataFile::CheckIntegrity can now be run without checking
   for missing instance files. This is used when removing a data dir (its
   instance file will be missing).
2. The DataDirManager and FsManager may now be constructed with both
   read_only=true and update_on_disk=true. The CLI tool leverages this
   functionality to create a "speculative" FsManager using the new data dir
   configuration that is used to check whether existing tablets depend on
   the data dir to be removed.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
10 files changed, 343 insertions(+), 90 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/3
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-10 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 2:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h@220
PS1, Line 220: the DataDirManager is
 :   // opened as per the new contents of of the provided 
directories but
> nit: extra "of"
Changed to:

  // If 'update_on_disk' and 'read_only' are both true, the directory manager
  // allows the provided directories to deviate from their on-disk data
  // structures without updating the latter to match the former.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h@223
PS1, Line 223:   bool update_on_disk;
> nit: should still note the default
Done


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@657
PS1, Line 657: Note: If 'has_missing_instance' is true, opts_.update_on_disk is
 :   // guaranteed to be true.
> nit: Maybe note that this is due to the ternary parameterization of CheckIn
Hmm, I originally included this so that it isn't confusing that L659-660 omits 
a check for opts_.update_on_disk.

I'll change it as per your first suggestion.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@805
PS1, Line 805: // This uuid's directory is missing outright, which can happen 
when
 : // opts_.read_only and opts_.update_on_disk are both 
true.
> After another look, I think this is ok.
OK, I took another stab at this comment. Let me know what you think.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/fs_manager.h@102
PS1, Line 102:   bool update_on_disk;
> nit: note the default
Done


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc@2232
PS1, Line 2232: ASSERT_OK(mts->WaitStarted());
> Could we check that we still have all the tablets we expected?
We could, but is it necessary? We wouldn't expect tablets to just disappear, 
right? We don't check them after adding a data directory above.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc@2240
PS1, Line 2240:   // Reconfigure the tserver to drop the data directory and 
restart it.
> nit: mind adding to the comment something along the lines of, "waiting for
Yeah, it's not intuitive that WaitStarted will return the first bootstrap 
failure. I'll note that.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc@854
PS1, Line 854: "Updates the set of data directories in an existing Kudu 
filesystem")
 :   .ExtraDescription("If a data directory is in use by a 
tablet and is "
 :   "removed, the operation will fail unless --force is 
also used")
 :   .AddOptionalParameter("fs_wal_dir")
 :   .AddOptionalParameter("fs_data_dirs")
 :   .AddOptionalParameter("force", boost::none, string("If 
true, permits "
 :   "the removal of a data directory that is configured 
for use by "
 :   "existing tablets. Those tablets will fail the next 
time the server "
 :   "is started"))
> micro-nit: I know in docs we favor being explicit, that local replicas are
I'm not sure; as you can see, List uses 'tablets' instead of 'local replicas'.



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 11 Jan 2018 00:13:40 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-10 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@805
PS1, Line 805: // This uuid's directory is missing outright, which can happen 
when
 : // opts_.read_only and opts_.update_on_disk are both 
true.
> So this can only happen when we open our "staging" FsManager and we're miss
After another look, I think this is ok.

Since we're only opening up the speculative FsManager, we shouldn't need to 
worry about potential mismatched UUIDs (e.g. if there's a missing directory AND 
a failed directory, we might end up assigning the failed directory the wrong 
UUID).

When we open the FsManager up for real, this shouldn't be the case because at 
that point, we'll update the path_sets and shouldn't reach this code block. Can 
you add a comment explaining why this missing directory doesn't matter?



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 10 Jan 2018 19:57:28 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-09 Thread Andrew Wong (Code Review)

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h
File src/kudu/fs/data_dirs.h:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h@220
PS1, Line 220: the DataDirManager is
 :   // opened as per the new contents of of the provided 
directories but
nit: extra "of"

Also it wasn't immediately clear what "new contents of the provided 
directories" was referring to. Maybe consider rewording "...the DataDirManager 
is opened to reflect the provided directories but..." or something? (in 
fs_manager.cc too)


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.h@223
PS1, Line 223:   bool update_on_disk;
nit: should still note the default


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@657
PS1, Line 657: Note: If 'has_missing_instance' is true, opts_.update_on_disk is
 :   // guaranteed to be true.
nit: Maybe note that this is due to the ternary parameterization of 
CheckIntegrity?

Or note why this is important, e.g. "...so we can go ahead an update the 
instance files".


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/data_dirs.cc@805
PS1, Line 805: // This uuid's directory is missing outright, which can happen 
when
 : // opts_.read_only and opts_.update_on_disk are both 
true.
So this can only happen when we open our "staging" FsManager and we're missing 
a directory?

I need to ponder this a bit more, not sure I've fully grokked this snippet.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/fs/fs_manager.h@102
PS1, Line 102:   bool update_on_disk;
nit: note the default


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc@2232
PS1, Line 2232: ASSERT_OK(mts->WaitStarted());
Could we check that we still have all the tablets we expected?


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/kudu-tool-test.cc@2240
PS1, Line 2240:   // Reconfigure the tserver to drop the data directory and 
restart it.
nit: mind adding to the comment something along the lines of, "waiting for the 
tablets to bootstrap should fail"?

WaitStarted() and Start() are so similar, I was a surprised for a second that 
WaitStarted() would fail until I realized that we're waiting for the bootstraps 
and not for the FsManager to start up.


http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/8978/1/src/kudu/tools/tool_action_fs.cc@854
PS1, Line 854: "Updates the set of data directories in an existing Kudu 
filesystem")
 :   .ExtraDescription("If a data directory is in use by a 
tablet and is "
 :   "removed, the operation will fail unless --force is 
also used")
 :   .AddOptionalParameter("fs_wal_dir")
 :   .AddOptionalParameter("fs_data_dirs")
 :   .AddOptionalParameter("force", boost::none, string("If 
true, permits "
 :   "the removal of a data directory that is configured 
for use by "
 :   "existing tablets. Those tablets will fail the next 
time the server "
 :   "is started"))
micro-nit: I know in docs we favor being explicit, that local replicas are 
called replicas and tablets are a logical concept. Does the same apply for 
tooling?



--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 10 Jan 2018 03:22:04 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-09 Thread Adar Dembo (Code Review)

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Patch Set 2: Verified+1

The test failures were a batch of clock synchronization errors on one 
distributed slave.


--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 10 Jan 2018 00:12:00 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-09 Thread Adar Dembo (Code Review)

Adar Dembo has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/8978 )

Change subject: KUDU-2202: support for removing data directories (take two)
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

2018-01-09 Thread Adar Dembo (Code Review)

Hello Andrew Wong, Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/8978

to review the following change.


Change subject: KUDU-2202: support for removing data directories (take two)
..

KUDU-2202: support for removing data directories (take two)

This commit extends "kudu fs update_dirs" to allow the removal of data
directories. A data dir may be removed provided there are no tablets
configured to use it. The --force flag may be used to override this safety
check; tablets that depend on the removed data dir will fail to bootstrap
the next time the server is started.

I'll be the first to admit that this functionality isn't terribly useful at
the moment because Kudu deployments are configured to stripe data across all
data directories, so removing a data dir will fail all tablets, which is
equivalent to reformatting the node. Nevertheless, it will become more
useful as users customize the new --fs_target_data_dirs_per_tablet gflag.

This is the second attempt at implementing this functionality. The first
used a different interface and was vulnerable to a data loss issue
documented in KUDU-2202. Andrew fixed that in commit 47b81c4, which reopens
the door for safely removing data directories.

I considered a stronger check that only prohibits data dir removal if there
were data blocks on the to-be-removed data dir, but decided against it:
- It's rare to find a tablet configured to use a data dir but without any
  actual blocks on it. In fact, that's only likely for empty tables, or
  tables with empty range partitions.
- We'd still need to rewrite any unaffected tablet superblocks and remove
  the data dir from their data dir groups.

There are several interesting modifications here:
1. PathInstanceMetadataFile::CheckIntegrity can now be run without checking
   for missing instance files. This is used when removing a data dir (its
   instance file will be missing).
2. The DataDirManager and FsManager may now be constructed with both
   read_only=true and update_on_disk=true. The CLI tool leverages this
   functionality to create a "speculative" FsManager using the new data dir
   configuration that is used to check whether existing tablets depend on
   the data dir to be removed.
3. The DataDirManager now checks for integrity after making on-disk changes.
   While not strictly necessary, this seems like a good sanity check to
   avoid introducing bugs.
4. The "kudu fs update_dirs" action was updated for data dir removal. It now
   accepts the --force option, without which removal will only be allowed if
   no tablets are configured to use the removed data dir.

Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
---
M src/kudu/fs/block_manager_util-test.cc
M src/kudu/fs/block_manager_util.cc
M src/kudu/fs/block_manager_util.h
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/fs/fs_manager.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
10 files changed, 317 insertions(+), 81 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/8978/1
--
To view, visit http://gerrit.cloudera.org:8080/8978
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iacb650115bcdf055542ef2777641a3201fc8e30b
Gerrit-Change-Number: 8978
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Todd Lipcon

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

[kudu-CR] KUDU-2202: support for removing data directories (take two)

27 matches

Site Navigation

Mail list logo

Footer information