You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "YangSong (Code Review)" <ge...@cloudera.org> on 2019/11/07 11:43:32 UTC
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
YangSong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14654
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
[KUDU-2975]Spread WAL across multiple directories
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-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/fs/fs_manager.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
27 files changed, 335 insertions(+), 100 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/1
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 1
Gerrit-Owner: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 13:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs.proto
File src/kudu/fs/fs.proto:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs.proto@58
PS13, Line 58: // A filesystem instance can contain multiple paths. One of these structures
: // is persisted in each path when the filesystem instance is created.
: message PathInstanceMetadataPB {
: // Describes this path instance as well as all of the other path instances
: // that, taken together, describe a complete set.
: required PathSetPB path_set = 1;
:
: // Textual representation of the block manager that formatted this path.
: required string block_manager_type = 2;
:
: // Block size on the filesystem where this instance was created. If the
: // instance (and its data) are ever copied to another location, the block
: // size in that location must be the same.
: required uint64 filesystem_block_size_bytes = 3;
: }
> I'm not convinced we have to reuse this either, especially if we end up not
If we don't neet to do the consistency check for WAL directories, we don't need to record the UUIDs of all WAL directories. If the "wal_manager_instance" file is not accessible, we can't know the UUID of this directory any more. We must make a new UUID for it. Maybe we just need to restore the file with the original UUID.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.h@98
PS13, Line 98: // The directory root where WALs will be stored. Cannot be empty.
: std::vector<std::string> wal_roots;
> Rather than keeping track of two sets of WAL roots (wal_root _and_ wal_root
Yes, we can put them in "wal_roots". Just make sure that only one gflag is used.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@195
PS13, Line 195: if (!opts_.wal_root.empty() && !opts_.wal_roots.empty()) {
: return Status::IOError("Write-ahead log directory (fs_wal_dir and fs_wal_dirs)"
: " provided conflictly");
: }
> nit: I think this would be nicer as a gflag validator.
I don't know how to use gflag validator. Here use "FLAGS_fs_wal_dir" and "FLAGS_fs_wal_dirs" instead of "opts_.wal_root" and "opts_.wal_roots"?
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@392
PS13, Line 392: if (opts_.consistency_check != ConsistencyCheckBehavior::UPDATE_ON_DISK) {
> Why this check?
If we use "fs_wal_dirs" instead of "fs_wal_dir", maybe we add a WAL dir, the new dir has not a subdir named "wals". The following code will report an error when we do "kudu fs update_dirs".
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@474
PS13, Line 474: report->wal_dir = canonicalized_wal_fs_roots_[0].path;
> We should probably allow the FsReport to accept multiple WAL directories to
Yes
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@534
PS13, Line 534: vector<string> ancillary_dirs = JoinPathSegmentsV(
: WalDirManager::GetRootNames(canonicalized_wal_fs_roots_), kWalDirName);
> How about let's have the WalDirManager manage the creation of WAL directori
Yes, it's required.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@735
PS13, Line 735:
: string FsManager::GetTabletWalDir(const std::string& tablet_id) const {
: string dir;
: Status s = wd_manager_->FindWalDirByTabletId(tablet_id, &dir);
: if (!s.ok()) {
: LOG(WARNING) << "cannot find tablet:" << tablet_id << " wal directory." <<
: s.message().ToString();
: return "";
: }
: return JoinPathSegments(dir, tablet_id);
: }
> I think we'll eventually want to have this return a Status and have a strin
Yes
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc
File src/kudu/fs/wal_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc@436
PS13, Line 436: consistency_check
> I'm not convinced that we need the consistency checking behavior for WALs.
this is for "kudu fs update_dirs" tool.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc@605
PS13, Line 605: std::lock_guard<percpu_rwlock> write_lock(dir_lock_);
> We only need to hold the lock in read mode for ContainsKey(), and we don't
OK
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tablet/tablet_bootstrap-test.cc@121
PS13, Line 121: fs_manager_->wd_manager()->DeleteWalDir(log::kTestTablet);
> Why did we have to do this?
This class is based on "LogTestBase", in "LogTestBase" SetUp() function, I add a line code like:
ASSERT_OK(fs_manager_->wd_manager()->CreateWalDir(kTestTablet));
so here I first delete the tablet's WAL dir message in WalDirManager. Let it created or loaded by "TabletMetadata::LoadOrCreate()".
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tools/tool_action_local_replica.cc@524
PS13, Line 524: CreateWalDir
> Aren't we operating on an existing tablet replica? Why are we creating a br
Yes, There's a problem here. I should load the tablet's metadata, and get the WAL dir from metedata, if cannot found, I should use "TryCreateWalDir". I will modify it later. Above is the same.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_copy_client-test.cc
File src/kudu/tserver/tablet_copy_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_copy_client-test.cc@298
PS13, Line 298: fs_manager_->wd_manager()->CreateWalDir(GetTabletId());
> Why are we creating a WAL directory from scratch here? Shouldn't the WAL di
Yes, I will modify it later.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_server-test.cc@2453
PS13, Line 2453: const int kLimit = rand() % (kMaxLimit + 1) + 1;
> This isn't related to this patch, right? If not, could you put it in a sepa
Yes, not related to this patch.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/util/env_util.cc@362
PS13, Line 362: Status ListDirsInDir(Env* env,
: const string& path,
: vector<string>* entries) {
: RETURN_NOT_OK(env->GetChildren(path, entries));
: auto iter = entries->begin();
: while (iter != entries->end()) {
: if (*iter == "." || *iter == ".." || iter->find(kTmpInfix) != string::npos) {
: iter = entries->erase(iter);
: continue;
: }
: bool is_dir = false;
: Status s = env->IsDirectory(JoinPathSegments(path, *iter), &is_dir);
: if (!s.ok() || !is_dir) {
: iter = entries->erase(iter);
: continue;
: }
: ++iter;
: }
: return Status::OK();
: }
:
> We don't need this anymore, right?
It's used in "mini_cluster_fs_inspector.cc". Go through all the files under the "wals" directory to get all tablets in old version, but new version we should go through the dirs, because we add a "wal_manager_instance" file under wals dir.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 13
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 15 Nov 2019 09:28:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 8:
(1 comment)
Compatibility is an issue right now. How to switch from the "--fs_wal_dir" configuration to the "--fs_wal_dirs"?
Compared to the new version, the old one has no "wal_manager_instance" file. That means the WAL directory has no UUID.
We may use "kudu fs update" tool to resolve it. Another problem is that tablet's metadata does not have "wal_dir" message.
The solution is to go through all the WAL directories, try to find the directory where the tablet located.
If found, load the tablet, otherwise mark the tablet failed. Above is my personal idea, I don't know if it will work.
http://gerrit.cloudera.org:8080/#/c/14654/8/src/kudu/fs/wal_dirs.h
File src/kudu/fs/wal_dirs.h:
http://gerrit.cloudera.org:8080/#/c/14654/8/src/kudu/fs/wal_dirs.h@177
PS8, Line 177: // Serializes the DataDirGroupPB associated with the given tablet_id.
Some comments in this file are wrong, I will modify it later.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 8
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Wed, 13 Nov 2019 07:17:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1508
PS3, Line 1508: void TSTabletManager::FailTabletsInDataDir(const string& uuid) {
: DataDirManager* dir_manager = nullptr;
: int uuid_idx;
: if (fs_manager_->dd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->dd_manager();
: } else if (fs_manager_->wd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->wd_manager();
: } else {
: LOG(FATAL) << Substitute("No directory found with UUID $0", uuid);
: }
: if (dir_manager->IsDataDirFailed(uuid_idx)) {
: LOG(WARNING) << "Data directory is already marked failed.";
: return;
: }
: // Fail the directory to prevent other tablets from being placed in it.
: dir_manager->MarkDataDirFailed(uuid_idx);
: set<string> tablets = dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx);
: LOG(INFO) << Substitute("Data dir $0 has $1 tablets", uuid, tablets.size());
: for (const string& tablet_id : dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx)) {
: FailTabletAndScheduleShutdown(tablet_id);
: }
: }
> Here I still don't understand how to keep a single FsErrorManager. In the i
I'm sorry, here I misunderstood. Always has two FsErrorManager.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 08 Nov 2019 06:00:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#21).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. Every WAL
directory has an UUID, We record the directory's UUID and all
directories's UUIDs into a file named "wal_manager_instance". We also record
the tablet's WAL directory UUID into the tablet's metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup.
Because the tablet's metadata in old version had no WAL directory, we
look for the tablet's WAL directory under all the new WAL directories.
If found, we write the talbet's WAL into WalDirManager. But flushing
metadata may causes a deadlock in the tool code, so we don't persist
the metadata immediately.
In this version, one of the WAL directorys's structure looks like this:
----wal
--------instance
--------wals
------------wal_manager_instance
------------tablet1_uuid
----------------index.0
----------------wal.0
------------tablet2_uuid
----------------index.0
----------------wal.0
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
58 files changed, 3,165 insertions(+), 322 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/21
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 21
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#18).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. We record
the tablet's WAL directory UUID into the metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup. Because the tablet's metadata in old version had
no WAL dir, we look for the tablet's WAL directory under all the new WAL
directories. If found, we write the talbet's WAL into WalDirManager. But
it's not persistent.
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory UUID form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
53 files changed, 2,809 insertions(+), 290 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/18
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 18
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#23).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. Every WAL
directory has an UUID, We record the directory's UUID and all
directories's UUIDs into a file named "wal_manager_instance". We also record
the tablet's WAL directory UUID into the tablet's metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup.
Because the tablet's metadata in old version had no WAL directory, we
look for the tablet's WAL directory under all the new WAL directories.
If found, we write the talbet's WAL into WalDirManager. But flushing
metadata may causes a deadlock in the tool code, so we don't persist
the metadata immediately.
In this version, one of the WAL directorys's structure looks like this:
----wal
--------instance
--------wals
------------wal_manager_instance
------------tablet1_uuid
----------------index.0
----------------wal.0
------------tablet2_uuid
----------------index.0
----------------wal.0
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M docs/administration.adoc
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/dir_util.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
59 files changed, 3,390 insertions(+), 323 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/23
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 23
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 3:
(7 comments)
I left some overall feedback. I'm curious if you and Adar agree/disagree. Basically, I think the concerns of the WALs and of the data are separate enough to warrant having a separate class for it.
Also we should think of how we want to test this:
- test the "happy" case with no disk failures. We should make sure that we get multiple WAL directories when we create new tablets.
- test common upgrade cases. What happens if we have an old --fs_wal_dir=/wal but now we supply --fs_wal_dirs=/wal ? Is that OK? How about if we supply --fs_wal_dirs=/wal,/wal2 or --fs_wal_dirs=/wal1,wal2 ?
- test what happens when we are missing WAL directories
- test what happens when we have more than one WAL directory for a given tablet (should be covered by having the UUID in the tablet metadata, but we should add a test for it)
http://gerrit.cloudera.org:8080/#/c/14654/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14654/3//COMMIT_MSG@7
PS3, Line 7: [KUDU-2975]Spread WAL across multiple directories
nit: for consistency with the rest of our commits, format as
KUDU-2975: Spread WAL across multiple directories
http://gerrit.cloudera.org:8080/#/c/14654/3//COMMIT_MSG@8
PS3, Line 8:
: Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Could you add a commit message explaining the design decisions made in this patch? Things like:
- This is reusing a new DataDirManager instance for WALs
- How (if at all) this handles missing WALs
- How (if at all) this handles failed WAL disks
- etc.
It's ok for some of the answers here to be that we don't handle certain edge cases yet, but it would make it easier to review if we know what to expect in this implementation.
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/fs/data_dirs.cc@723
PS3, Line 723: if (opts_.dir_type == "data") {
: // Create a per-dir thread pool.
This style of checking makes the code more difficult to understand and maintain, which is why I think the WalDirManager should be its own class. It seems like the WalDirManager would only need to implement a subset of the functionality of the DataDirManager.
It doesn't have to worry about:
* per-data-dir work done in threadpools
* anything related to groups of directories (since there's only one WAL directory per tablet)
* returning a subset of a directory group (like GetDirForBlock())
* I don't think we need to worry about checking the path instance UUIDs. This seemed important for data because tablet data could be spread across multiple data directories, so opening with a subset of directories could result in scenarios where some blocks exist and some don't. This has gotten even better with data directory disk failure handling. With WALs, if we open with a subset of the directories, I think it might not be unreasonable to not simply fail the tablet if missing a WAL directory. On the other hand, I could see this being useful to ensure that there are no errors when inputting the gflags, so I could be convinced that this is important.
That means we _do_ have to worry about:
* a mapping from tablet ID to WAL directory index
* a mapping from WAL directory index to a set of tablet IDs
* registering a single WAL directory when creating a new tablet
* marking a WAL directory as failed
But I believe the implementation for these will be simpler because each tablet will only have a single WAL directory.
I'm curious whether you and Adar agree on this. I can be convinced that this approach is OK too, if there are good reasons for keeping DataDirManager and WALs tightly coupled like this.
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tablet/metadata.proto
File src/kudu/tablet/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tablet/metadata.proto@150
PS3, Line 150: // Tablet WAL is spread across multi directories. If this is not set,
: // it is assumed that the WAL is from a version of Kudu before 1.10.0.
: // In this case, WAL will be created spanning only one directory.
: optional DataDirGroupPB wal_dir_group = 19;
Do we actually want to use a group? I thought we would only want to use a single WAL directory per tablet, given the Log class is single-threaded per tablet.
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/tablet_server-test.cc@2453
PS3, Line 2453: const int kLimit = rand() % (kMaxLimit + 1) + 1;
Why did this change?
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1471
PS3, Line 1471: // This place is not sure if there is a better way to modify it.
: meta->fs_manager()->wd_manager()->DeleteDataDirGroup(tablet_id);
: RETURN_NOT_OK(meta->Flush());
Rather than including this here (and doing another Flush()), could we move the log deletion up to before we delete the metadata?
Then we could put this into the metadata->DeleteTabletData() call.
An edge case to think about is what happens when the WAL deletion succeeds but the metadata deletion fails?
Before, that would be OK because if the data deletion failed, we can assume that the data state is OK, and we didn't delete our WALs. If we moved this up, we might end up deleting our WALs but failing to delete the data. This seems like it might be OK though because we should be able to handle the case where we don't have a WAL, but we do have data/metadata -- we should just fail the tablet.
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1508
PS3, Line 1508: void TSTabletManager::FailTabletsInDataDir(const string& uuid) {
: DataDirManager* dir_manager = nullptr;
: int uuid_idx;
: if (fs_manager_->dd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->dd_manager();
: } else if (fs_manager_->wd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->wd_manager();
: } else {
: LOG(FATAL) << Substitute("No directory found with UUID $0", uuid);
: }
: if (dir_manager->IsDataDirFailed(uuid_idx)) {
: LOG(WARNING) << "Data directory is already marked failed.";
: return;
: }
: // Fail the directory to prevent other tablets from being placed in it.
: dir_manager->MarkDataDirFailed(uuid_idx);
: set<string> tablets = dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx);
: LOG(INFO) << Substitute("Data dir $0 has $1 tablets", uuid, tablets.size());
: for (const string& tablet_id : dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx)) {
: FailTabletAndScheduleShutdown(tablet_id);
: }
: }
Rather than reusing this exactly, I'd prefer we NOT tightly couple the handling together. Maybe separate this out into a
TSTabletManager::FailTabletsInWalDir(const string& uuid)
If we did that, we could keep a single FsErrorManager, right?
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 08 Nov 2019 02:17:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#20).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. Every WAL
directory has an UUID, We record the directory's UUID and all
directories's UUIDs into a file named "wal_manager_instance". We also record
the tablet's WAL directory UUID into the tablet's metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup.
Because the tablet's metadata in old version had no WAL directory, we
look for the tablet's WAL directory under all the new WAL directories.
If found, we write the talbet's WAL into WalDirManager. But flushing
metadata may causes a deadlock in the tool code, so we don't persist
the metadata immediately.
In this version, one of the WAL directorys's structure looks like this:
----wal
--------instance
--------wals
------------wal_manager_instance
------------tablet1_uuid
----------------index.0
----------------wal.0
------------tablet2_uuid
----------------index.0
----------------wal.0
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
Deleting tablet has also been modified, we put the delete WAL before the
delete metadata. Here may be not safe, need discuss.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
58 files changed, 3,217 insertions(+), 323 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/20
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 20
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#4).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
33 files changed, 1,355 insertions(+), 86 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/4
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 4
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1508
PS3, Line 1508: void TSTabletManager::FailTabletsInDataDir(const string& uuid) {
: DataDirManager* dir_manager = nullptr;
: int uuid_idx;
: if (fs_manager_->dd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->dd_manager();
: } else if (fs_manager_->wd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->wd_manager();
: } else {
: LOG(FATAL) << Substitute("No directory found with UUID $0", uuid);
: }
: if (dir_manager->IsDataDirFailed(uuid_idx)) {
: LOG(WARNING) << "Data directory is already marked failed.";
: return;
: }
: // Fail the directory to prevent other tablets from being placed in it.
: dir_manager->MarkDataDirFailed(uuid_idx);
: set<string> tablets = dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx);
: LOG(INFO) << Substitute("Data dir $0 has $1 tablets", uuid, tablets.size());
: for (const string& tablet_id : dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx)) {
: FailTabletAndScheduleShutdown(tablet_id);
: }
: }
> Are the UUIDs in the WAL directories the same as the UUIDs of the data dire
I known what you mean, yes, they can share a single FsErrorManager.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 08 Nov 2019 06:27:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 20:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1471
PS3, Line 1471: MAYBE_FAULT(FLAGS_fault_crash_after_blocks_deleted);
:
: // We do not delete the super
> Rather than including this here (and doing another Flush()), could we move
I'm not sure it's safe. if a tablet has just been created, and we write two records into it, the delete it. that mean there is no data in the disk, all data is in MemRowSet. What will happen when the WAL deletion succeeds but the metadata deletion fails?
If the tablet's WAL directory is deleted, we will find the tablet has no data blocks and no WAL, It will be reinitialized as a new tablet. I have no way to distinguish this exception with creating a new tablet.
Another case, if just a segment is deleted, I have not read the replaying log code deeply. I'm not sure Kudu can check out this exception. Maybe the index file can tell out this exception.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 20
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Wed, 27 Nov 2019 08:55:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#17).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. We record
the tablet's WAL directory UUID into the metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup. Because the tablet's metadata in old version had
no WAL dir, we look for the tablet's WAL directory under all the new WAL
directories. If found, we write the talbet's WAL into WalDirManager. But
it's not persistent.
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory UUID form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
53 files changed, 2,808 insertions(+), 290 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/17
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 17
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 22:
(23 comments)
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.cc@84
PS22, Line 84:
: DEFINE_string(fs_wal_dir, "",
: "Directory with write-ahead logs. If this is not specified, the "
: "program will not start. May be the same as fs_data_dirs");
: TAG_FLAG(fs_wal_dir, stable);
: DEFINE_string(fs_wal_dirs, "",
: "Directory with write-ahead logs. If this is not specified, the "
: "program will not start. May be the same as fs_data_dirs");
: TAG_FLAG(fs_wal_dirs, stable);
> Can you add a group flag validator like ValidateUpdateTabletStatsInterval i
I don't know how to define this place. I try to define it as:
bool ValidateFsWalDir() {
bool has_a = !FLAGS_fs_wal_dir.empty();
bool has_b = !FLAGS_fs_wal_dirs.empty();
if (has_a == has_b) {
LOG(ERROR) << "--fs_wal_dir and --fs_wal_dirs only one can be set.";
return false;
}
return true;
}
GROUP_FLAG_VALIDATOR(fs_wal_dir, ValidateFsWalDir);
but I found It's not the concept "group", they are mutually exclusive. If I define like this, the many tests cannot run, such as fs_manager-test.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.cc@735
PS22, Line 735: Status FsManager::GetOrphanedWalDir(const string& tablet_id, string* wal_dir) {
: if (wal_dir == nullptr) {
: return Status::InvalidArgument("wal_dir cannot be nullptr");
: }
: Status s = wd_manager_->FindTabletDirFromDisk(tablet_id, "", wal_dir);
: if (!s.ok()) {
: LOG(WARNING) << "cannot find tablet:" << tablet_id << " wal directory." <<
: s.message().ToString();
: }
: return s;
: }
:
: Status FsManager::GetOrphanedWalRecoveryDir(const string& tablet_id,
: string* wal_recovery_dir) {
: if (wal_recovery_dir == nullptr) {
: return Status::InvalidArgument("wal_recovery_dir cannot be nullptr");
: }
: Status s = wd_manager_->FindTabletDirFromDisk(tablet_id, kWalsRecoveryDirSuffix,
: wal_recovery_dir);
: if (!s.ok()) {
: LOG(WARNING) << "cannot find tablet:" << tablet_id << " wal directory." <<
: s.message().ToString();
: }
: return s;
: }
:
> As written, I think these mean that:
Yes, because we delete metadata first then delete log when delete tablet, so maybe have some orphaned WAL dir. We should find this orphaned WAL dir then delete them at the next time it starts. When we determine if there is tablet WAL data, we look at the tablet's metadata first then look it up on the WAL dirs.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc
File src/kudu/fs/wal_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@75
PS22, Line 75: DEFINE_bool(fs_wal_dirs_consider_available_space, true,
: "Whether to consider available space when selecting a wal "
: "directory during tablet or wal block creation.");
: TAG_FLAG(fs_wal_dirs_consider_available_space, runtime);
: TAG_FLAG(fs_wal_dirs_consider_available_space, evolving);
> This isn't used, is it?
Yes.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@605
PS22, Line 605: r *wal_
> nit: move the * so it's next to the type name, i.e.
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@614
PS22, Line 614: if (other != nullptr) {
> nit: this can be written as:
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@616
PS22, Line 616: "tried to load directory for tablet $0 but one is already registered",
: tablet_id));
> nit: maybe also log the existing directory?
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@664
PS22, Line 664: postfix
> nit: we usually call this a "suffix"
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@675
PS22, Line 675: if (!postfix.empty()) {
: tablet_path.append(postfix);
: }
> We don't need to condition on postfix.empty(). If it's empty, tablet_path.a
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@710
PS22, Line 710: if (tablets_by_uuid.second.size() < min_tablets_num) {
: min_tablets_num = tablets_by_uuid.second.size();
: target_uuid = tablets_by_uuid.first;
: }
> The fact that we're always using the least-populated directory is different
Yes, The current algorithm is relatively simple, just make sure that each WAL directory has the same number of tablets.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@808
PS22, Line 808:
: bool WalDirManager::IsWalDirFailed(const string& uuid) const {
: shared_lock<rw_spinlock> lock(dir_lock_.get_lock());
: return ContainsKey(failed_wal_dirs_, uuid);
: }
> This isn't used, right? Same with MarkWalDirFailedByUuid
yes.MarkWalDirFailedByUuid is not used right now, may be used in the future in case of disk failure.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata-test.cc
File src/kudu/tablet/tablet_metadata-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata-test.cc@47
PS22, Line 47: class TestTabletMetadata : public KuduTabletTest {
: public:
: TestTabletMetadata()
: : KuduTabletTest(GetSimpleTestSchema()) {
: }
:
: virtual void SetUp() OVERRIDE {
: KuduTabletTest::SetUp();
: writer_.reset(new LocalTabletWriter(harness_->tablet().get(),
: &client_schema_));
: }
:
: void BuildPartialRow(int key, int intval, const char* strval,
: gscoped_ptr<KuduPartialRow>* row);
:
: protected:
: gscoped_ptr<LocalTabletWriter> writer_;
: };
> TODO(awong): can we also add a test that, if we load a superblock that _doe
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata.cc@463
PS22, Line 463: // An error loading the WAL dir is non-fatal, it just means the
: // tablet will fail to bootstrap later.
> TODO(awong): make sure this is true
I will add some test.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/kudu-tool-test.cc@5219
PS22, Line 5219: // master use "fs_wal_dir", tserver use "fs_wal_dirs".
> Is there a good reason to do this here? I know that in a real deployment, w
In the ExternalMiniCluster, master keep using "fs_wal_dir", because master has only one tablet, using "fs_wal_dirs" is pointless. And tserver change to use "fs_wal_dirs". I will change the ExternalMiniCluster to always use fs_wal_dirs.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_fs.cc@342
PS22, Line 342: // data dir group for the tablet, and the staged directory config won't
: // affect this tablet.
: DCHECK(s1.IsNotFound() || s2.IsNotFound()) << s1.ToString();
: continue;
: }
: RETURN_NOT_OK_PREPEND(s1, "at least one tablet is configured to use removed data directory. "
: "Retry with --force to override this");
: RETURN_NOT_OK_PREPEND(s2, "at least one tablet is configured to use removed WAL directory. "
: "Retry with --force to override this");
: }
> TODO(awong): is this tested
I tested it by hand. I will add some test code.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_local_replica.cc@367
PS22, Line 367: // For new version, need load metadata, find directory from metadata. But we just find from
: // wal directory first.
: RETURN_NOT_OK(fs_manager.wd_manager()->FindAndRegisterWalDir(tablet_id));
> Doesn't this already happen in TabletMetadata::Load()?
Yes.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_local_replica.cc@524
PS22, Line 524: // For new version, need load metadata, find directory from metadata.
: // But we just find the wal dir for tablet from disk first.
: RETURN_NOT_OK(fs_manager->wd_manager()->FindAndRegisterWalDir(tablet_id));
> Should we call TabletMetadata::Load() instead?
Yes.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server-test.cc
File src/kudu/tserver/mini_tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server-test.cc@41
PS22, Line 41:
: TEST_F(MiniTabletServerTest, TestMultiDataDirServer) {
: // Specifying the number of data directories will create subdirectories under the test root.
: unique_ptr<MiniTabletServer> mini_server;
: FsManager* fs_manager;
:
: int kNumDataDirs = 3;
: mini_server.reset(new MiniTabletServer(GetTestPath("TServer"),
: HostPort("127.0.0.1", 0), kNumDataDirs));
: ASSERT_OK(mini_server->Start());
: fs_manager = mini_server->server()->fs_manager();
: ASSERT_EQ(1, fs_manager->GetWalRootDirs().size());
: ASSERT_STR_CONTAINS(DirName(fs_manager->GetWalRootDirs()[0]), "wal");
: ASSERT_EQ(kNumDataDirs, fs_manager->GetDataRootDirs().size());
: }
:
: TEST_F(MiniTabletServerTest, TestMultiWalDirServer) {
: // Specifying the number of data directories will create subdirectories under the test root.
: unique_ptr<MiniTabletServer> mini_server;
: FsManager* fs_manager;
:
: int kNumWalDirs = 3;
: mini_server.reset(new MiniTabletServer(GetTestPath("TServer"),
: HostPort("127.0.0.1", 0), 1, kNumWalDirs));
: ASSERT_OK(mini_server->Start());
: fs_manager = mini_server->server()->fs_manager();
: ASSERT_EQ(1, fs_manager->GetDataRootDirs().size());
: ASSERT_STR_CONTAINS(DirName(fs_manager->GetDataRootDirs()[0]), "data");
: ASSERT_EQ(kNumWalDirs, fs_manager->GetWalRootDirs().size());
: }
> nit: maybe combine these tests so we have a single test that has multiple W
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server.cc
File src/kudu/tserver/mini_tablet_server.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server.cc@76
PS22, Line 76: // This place needs to be consistent with the original logic.
: // Otherwise, some test cases will not pass.
: // If there just one wal dir and data dir, we just use fs_root.
> nit: Thanks for the note! This comment should be removed before we merge th
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_copy_client-test.cc
File src/kudu/tserver/tablet_copy_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_copy_client-test.cc@507
PS22, Line 507: fs_manager_->wd_manager()->CreateWalDir(GetTabletId());
> Why do we need this? Shouldn't this have already happened when we setup the
We need prepare a WAL dir for tablet. when we setup the test, we just init the fs_manager. this is done in StartCopy(). This case has not used "StartCopy()", so we need do it here.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-stress-test.cc
File src/kudu/tserver/tablet_server-stress-test.cc:
PS22:
> Instead of copying the existing test, how about making this a parameterized
OK
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-test.cc@206
PS22, Line 206: /*num_data_dirs=*/1, /*num_wal_dirs=*/3
> Why not leave it using the default of 1 data directory and 1 WAL directory?
Here I just want to test multiple WAL dirs. I can change back if it's not necessary.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-test.cc@3735
PS22, Line 3735: string tablet_meta_path = JoinPathSegments(JoinPathSegments(
: GetTestPath("TabletServerTest-fsroot"), "wal-0"), "tablet-meta");
> I'm kind of surprised by this. Why did the metadata directory change?
Because there are 3 WAL directories, in function MiniTabletServer::MiniTabletServer(), we named the wal directory name as "wal-0" "wal-1" "wal-2", in fs_manager, the metadata directory use the first wal directory, so here the metadata directory name change to "wal-0". The original code had only one wal directory and one data directory, so it use "TabletServerTest-fsroot" as the wal and metadata directory.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/ts_tablet_manager.cc@1460
PS22, Line 1460: meta->fs_manager()->GetTabletWalDir(tablet_id, &wal_dir);
: meta->fs_manager()->GetTabletWalRecoveryDir(tablet_id, &wal_recovery_dir);
> Should we WARN_NOT_OK here? Or maybe we should check to look if the status
yes, we should WARN_NOT_OK. there is a case, if metadata deletion succeed but log deletion failed, we cannot get the tablet's dir, but Log::DeleteOnDiskData() also can delete it by finding the tablet's dir from disk. So I think here just need a warning.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 22
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Thu, 12 Dec 2019 03:20:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#13).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. We record
the tablet's WAL directory UUID into the metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup.
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the
following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory UUID form metadata
3. if WAL dir UUID missing in tablet's metadata, how to find the UUID,
and flush to metadata.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
32 files changed, 1,418 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/13
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 13
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#10).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
32 files changed, 1,375 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/10
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 10
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#14).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. We record
the tablet's WAL directory UUID into the metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup. Because the tablet's metadata in old version had
no WAL dir, we look for the tablet's WAL directory under all the new WAL
directories. If found, we write the talbet's WAL into WalDirManager. But
it's not persistent.
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory UUID form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
52 files changed, 2,741 insertions(+), 277 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/14
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 14
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#6).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
33 files changed, 1,356 insertions(+), 87 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/6
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 6
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 22:
> Patch Set 22:
>
> (30 comments)
Oops, I left in some notes to myself (TODO(awong)). Most of the TODOs are questions about whether certain scenarios are tested. If any aren't, it'd be great if you could add tests for what I described.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 22
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Wed, 11 Dec 2019 01:20:07 +0000
Gerrit-HasComments: No
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 1:
Spreading WAL accross multiple dirs works well in the local test. There are still these problems: during switch "fs_wal_dir" to "fs_wal_dirs", not compatible. WAL disk failure not support. Tool about wal not modify. Lack tests.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 1
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Thu, 07 Nov 2019 11:53:00 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#24).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. Every WAL
directory has an UUID, We record the directory's UUID and all
directories's UUIDs into a file named "wal_manager_instance". We also record
the tablet's WAL directory UUID into the tablet's metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup.
Because the tablet's metadata in old version had no WAL directory, we
look for the tablet's WAL directory under all the new WAL directories.
If found, we write the talbet's WAL into WalDirManager. But flushing
metadata may causes a deadlock in the tool code, so we don't persist
the metadata immediately.
In this version, one of the WAL directorys's structure looks like this:
----wal
--------instance
--------wals
------------wal_manager_instance
------------tablet1_uuid
----------------index.0
----------------wal.0
------------tablet2_uuid
----------------index.0
----------------wal.0
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M docs/administration.adoc
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/dir_util.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
60 files changed, 3,399 insertions(+), 335 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/24
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 24
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 22:
(30 comments)
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs.proto
File src/kudu/fs/fs.proto:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs.proto@121
PS22, Line 121: // Tablet log is spread across a specified WAL directorie. This PB is represented
: // by the UUID of the WAL directorie it consists of.
nit: maybe to make it clearer that a single tablet will have a single WAL directory, reword this as:
"A tablet's WAL exists in a single WAL directory. This directory is represented by the UUID."
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.h@218
PS22, Line 218:
: Status GetOrphanedWalDir(const std::string& tablet_id, std::string* wal_dir);
:
: Status GetOrphanedWalRecoveryDir(const std::string& tablet_id,
: std::string* wal_recovery_dir);
The idea of "orphaned" WAL directories is new in this patch. Could you comment what these mean? When would we expect there to be an orphaned WAL directory? Or an orphaned recovery directory?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.cc@84
PS22, Line 84:
: DEFINE_string(fs_wal_dir, "",
: "Directory with write-ahead logs. If this is not specified, the "
: "program will not start. May be the same as fs_data_dirs");
: TAG_FLAG(fs_wal_dir, stable);
: DEFINE_string(fs_wal_dirs, "",
: "Directory with write-ahead logs. If this is not specified, the "
: "program will not start. May be the same as fs_data_dirs");
: TAG_FLAG(fs_wal_dirs, stable);
Can you add a group flag validator like ValidateUpdateTabletStatsInterval in ts_tablet_manager.cc that verifies that exactly one of these is empty? That should simplify some of the behavior below.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/fs_manager.cc@735
PS22, Line 735: Status FsManager::GetOrphanedWalDir(const string& tablet_id, string* wal_dir) {
: if (wal_dir == nullptr) {
: return Status::InvalidArgument("wal_dir cannot be nullptr");
: }
: Status s = wd_manager_->FindTabletDirFromDisk(tablet_id, "", wal_dir);
: if (!s.ok()) {
: LOG(WARNING) << "cannot find tablet:" << tablet_id << " wal directory." <<
: s.message().ToString();
: }
: return s;
: }
:
: Status FsManager::GetOrphanedWalRecoveryDir(const string& tablet_id,
: string* wal_recovery_dir) {
: if (wal_recovery_dir == nullptr) {
: return Status::InvalidArgument("wal_recovery_dir cannot be nullptr");
: }
: Status s = wd_manager_->FindTabletDirFromDisk(tablet_id, kWalsRecoveryDirSuffix,
: wal_recovery_dir);
: if (!s.ok()) {
: LOG(WARNING) << "cannot find tablet:" << tablet_id << " wal directory." <<
: s.message().ToString();
: }
: return s;
: }
:
As written, I think these mean that:
- a WAL directory hasn't already been registered with the WAL directory manager
- we'll search for a WAL directory from disk.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.h
File src/kudu/fs/wal_dirs.h:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.h@210
PS22, Line 210: // Results in an error if the tablet has not a WAL dir associated with
: // it. If returning with an error, the
: // WalDirManager will be unchanged.
nit: reword and add a bit extra:
"Looks into each WAL directory for an existing WAL for 'tablet_id'. Results in an error if no such WAL exists, or if there are multiple associated with the given tablet ID."
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.h@353
PS22, Line 353: dir_by_tablet_map_
nit: can you rename this so it's clear whether this is referring to the UUID or the directory name? Like 'dir_name_by_tablet_' or 'uuid_by_tablet_'
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc
File src/kudu/fs/wal_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@75
PS22, Line 75: DEFINE_bool(fs_wal_dirs_consider_available_space, true,
: "Whether to consider available space when selecting a wal "
: "directory during tablet or wal block creation.");
: TAG_FLAG(fs_wal_dirs_consider_available_space, runtime);
: TAG_FLAG(fs_wal_dirs_consider_available_space, evolving);
This isn't used, is it?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@605
PS22, Line 605: r *wal_
nit: move the * so it's next to the type name, i.e.
const WalDir* wal_dir = ...
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@614
PS22, Line 614: if (other != nullptr) {
nit: this can be written as:
if (other) {
...
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@616
PS22, Line 616: "tried to load directory for tablet $0 but one is already registered",
: tablet_id));
nit: maybe also log the existing directory?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@664
PS22, Line 664: postfix
nit: we usually call this a "suffix"
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@675
PS22, Line 675: if (!postfix.empty()) {
: tablet_path.append(postfix);
: }
We don't need to condition on postfix.empty(). If it's empty, tablet_path.append(postfix) will be a no-op.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@710
PS22, Line 710: if (tablets_by_uuid.second.size() < min_tablets_num) {
: min_tablets_num = tablets_by_uuid.second.size();
: target_uuid = tablets_by_uuid.first;
: }
The fact that we're always using the least-populated directory is different than what we do for data directories. Is that intentional? Why did you choose this heuristic instead of the PO2C algorithm?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/fs/wal_dirs.cc@808
PS22, Line 808:
: bool WalDirManager::IsWalDirFailed(const string& uuid) const {
: shared_lock<rw_spinlock> lock(dir_lock_.get_lock());
: return ContainsKey(failed_wal_dirs_, uuid);
: }
This isn't used, right? Same with MarkWalDirFailedByUuid
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata-test.cc
File src/kudu/tablet/tablet_metadata-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata-test.cc@47
PS22, Line 47: class TestTabletMetadata : public KuduTabletTest {
: public:
: TestTabletMetadata()
: : KuduTabletTest(GetSimpleTestSchema()) {
: }
:
: virtual void SetUp() OVERRIDE {
: KuduTabletTest::SetUp();
: writer_.reset(new LocalTabletWriter(harness_->tablet().get(),
: &client_schema_));
: }
:
: void BuildPartialRow(int key, int intval, const char* strval,
: gscoped_ptr<KuduPartialRow>* row);
:
: protected:
: gscoped_ptr<LocalTabletWriter> writer_;
: };
TODO(awong): can we also add a test that, if we load a superblock that _doesn't_ have a WAL UUID, then we'll assign one based on what we find on disk? And if that fails, then we'll get an error.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata.cc@463
PS22, Line 463: // An error loading the WAL dir is non-fatal, it just means the
: // tablet will fail to bootstrap later.
TODO(awong): make sure this is true
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata.cc@470
PS22, Line 470: 1.10.0
before 1.11, since this new code would land in 1.12
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata.cc@477
PS22, Line 477: // Here cannot flush directly. May causes a deadlock in the tool code.
: // If we switch 'fs_wal_dir' to 'fs_wal_dirs', we should try to find the directory
: // where tablet's WAL is located.
: // If we found, we should record WAL directory UUID into metadata, and flush metadata.
: // otherwise we do nothing here. At bootstrap we will check the tablet's WAL directory.
: // And it will failed, this tablet will be marked failed.
: //RETURN_NOT_OK(Flush());
Do we need this anymore?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tablet/tablet_metadata.cc@741
PS22, Line 741: DataDirGroupPB
nit: extend this, like:
"Serialize the tablet's data directory group and WAL directory, if they exist. They may.."
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/kudu-tool-test.cc@5219
PS22, Line 5219: // master use "fs_wal_dir", tserver use "fs_wal_dirs".
Is there a good reason to do this here? I know that in a real deployment, we'd probably use a single WAL directory. But this test doesn't really have much to do with the fs_wal_dir flags specifically; --fs_wal_dir and --fs_wal_dirs aren't special when it comes to gflags. Could we just leave it as is? Or could we change the ExternalMiniCluster to always use fs_wal_dirs, even if the number of WAL directories is 1?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_fs.cc@342
PS22, Line 342: // data dir group for the tablet, and the staged directory config won't
: // affect this tablet.
: DCHECK(s1.IsNotFound() || s2.IsNotFound()) << s1.ToString();
: continue;
: }
: RETURN_NOT_OK_PREPEND(s1, "at least one tablet is configured to use removed data directory. "
: "Retry with --force to override this");
: RETURN_NOT_OK_PREPEND(s2, "at least one tablet is configured to use removed WAL directory. "
: "Retry with --force to override this");
: }
TODO(awong): is this tested
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_local_replica.cc@367
PS22, Line 367: // For new version, need load metadata, find directory from metadata. But we just find from
: // wal directory first.
: RETURN_NOT_OK(fs_manager.wd_manager()->FindAndRegisterWalDir(tablet_id));
Doesn't this already happen in TabletMetadata::Load()?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tools/tool_action_local_replica.cc@524
PS22, Line 524: // For new version, need load metadata, find directory from metadata.
: // But we just find the wal dir for tablet from disk first.
: RETURN_NOT_OK(fs_manager->wd_manager()->FindAndRegisterWalDir(tablet_id));
Should we call TabletMetadata::Load() instead?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server-test.cc
File src/kudu/tserver/mini_tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server-test.cc@41
PS22, Line 41:
: TEST_F(MiniTabletServerTest, TestMultiDataDirServer) {
: // Specifying the number of data directories will create subdirectories under the test root.
: unique_ptr<MiniTabletServer> mini_server;
: FsManager* fs_manager;
:
: int kNumDataDirs = 3;
: mini_server.reset(new MiniTabletServer(GetTestPath("TServer"),
: HostPort("127.0.0.1", 0), kNumDataDirs));
: ASSERT_OK(mini_server->Start());
: fs_manager = mini_server->server()->fs_manager();
: ASSERT_EQ(1, fs_manager->GetWalRootDirs().size());
: ASSERT_STR_CONTAINS(DirName(fs_manager->GetWalRootDirs()[0]), "wal");
: ASSERT_EQ(kNumDataDirs, fs_manager->GetDataRootDirs().size());
: }
:
: TEST_F(MiniTabletServerTest, TestMultiWalDirServer) {
: // Specifying the number of data directories will create subdirectories under the test root.
: unique_ptr<MiniTabletServer> mini_server;
: FsManager* fs_manager;
:
: int kNumWalDirs = 3;
: mini_server.reset(new MiniTabletServer(GetTestPath("TServer"),
: HostPort("127.0.0.1", 0), 1, kNumWalDirs));
: ASSERT_OK(mini_server->Start());
: fs_manager = mini_server->server()->fs_manager();
: ASSERT_EQ(1, fs_manager->GetDataRootDirs().size());
: ASSERT_STR_CONTAINS(DirName(fs_manager->GetDataRootDirs()[0]), "data");
: ASSERT_EQ(kNumWalDirs, fs_manager->GetWalRootDirs().size());
: }
nit: maybe combine these tests so we have a single test that has multiple WAL directories and multiple data directories?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server.cc
File src/kudu/tserver/mini_tablet_server.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/mini_tablet_server.cc@76
PS22, Line 76: // This place needs to be consistent with the original logic.
: // Otherwise, some test cases will not pass.
: // If there just one wal dir and data dir, we just use fs_root.
nit: Thanks for the note! This comment should be removed before we merge this.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_copy_client-test.cc
File src/kudu/tserver/tablet_copy_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_copy_client-test.cc@507
PS22, Line 507: fs_manager_->wd_manager()->CreateWalDir(GetTabletId());
Why do we need this? Shouldn't this have already happened when we setup the test?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-stress-test.cc
File src/kudu/tserver/tablet_server-stress-test.cc:
PS22:
Instead of copying the existing test, how about making this a parameterized test, parameterized by the number of directories to use.
Search around for "TEST_P" and "INSTANTIATE_TEST_CASE_P" for examples of parameterized tests.
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-test.cc@206
PS22, Line 206: /*num_data_dirs=*/1, /*num_wal_dirs=*/3
Why not leave it using the default of 1 data directory and 1 WAL directory?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/tablet_server-test.cc@3735
PS22, Line 3735: string tablet_meta_path = JoinPathSegments(JoinPathSegments(
: GetTestPath("TabletServerTest-fsroot"), "wal-0"), "tablet-meta");
I'm kind of surprised by this. Why did the metadata directory change?
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/22/src/kudu/tserver/ts_tablet_manager.cc@1460
PS22, Line 1460: meta->fs_manager()->GetTabletWalDir(tablet_id, &wal_dir);
: meta->fs_manager()->GetTabletWalRecoveryDir(tablet_id, &wal_recovery_dir);
Should we WARN_NOT_OK here? Or maybe we should check to look if the status is an error and LOG(FATAL) if so?
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 22
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Wed, 11 Dec 2019 01:10:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#12).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
32 files changed, 1,417 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/12
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 12
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1508
PS3, Line 1508: void TSTabletManager::FailTabletsInDataDir(const string& uuid) {
: DataDirManager* dir_manager = nullptr;
: int uuid_idx;
: if (fs_manager_->dd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->dd_manager();
: } else if (fs_manager_->wd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->wd_manager();
: } else {
: LOG(FATAL) << Substitute("No directory found with UUID $0", uuid);
: }
: if (dir_manager->IsDataDirFailed(uuid_idx)) {
: LOG(WARNING) << "Data directory is already marked failed.";
: return;
: }
: // Fail the directory to prevent other tablets from being placed in it.
: dir_manager->MarkDataDirFailed(uuid_idx);
: set<string> tablets = dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx);
: LOG(INFO) << Substitute("Data dir $0 has $1 tablets", uuid, tablets.size());
: for (const string& tablet_id : dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx)) {
: FailTabletAndScheduleShutdown(tablet_id);
: }
: }
> I'm sorry, here I misunderstood. Always has two FsErrorManager.
Are the UUIDs in the WAL directories the same as the UUIDs of the data directories though? I thought they wouldn't be because they're completely separate entities, and they don't share the same block_manager_instance files.
I would think that if we failed to write to the WAL, we would trigger failure handling for that WAL directory (e.g. if we know that we failed to write to a tablet's WAL, maybe we would lookup the tablet's WAL directory and then fail _all_ the tablets in that WAL directory). We should be able to do this completely separately from the data directory path, which will trigger failure handling when a block fails to write/read.
Because the handling is separate, we could probably separate the DISK_ERROR enum to DATA_DISK_ERROR and WAL_DISK_ERROR enums, and then share a single FsErrorManager.
I also think that for initial implementation, we should not focus on disk failure handling in the WALs at all, and just focus on the structures. We can always improve in later patches.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 08 Nov 2019 06:11:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 3:
(2 comments)
This modification is general. So I haven't written test code yet. And there are still these problems.
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
I agree that the implementation for WAL manager is much simpler than data manager. They do almost same things in initialization, make dir and create 'block_manager_instance'. This is their biggest commonality. And I find some thing wrong in DataDirManager::Create().
Having a separate class for WAL manager is my original idea. But now I'm not sure which method is better.
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/tablet_server-test.cc@2453
PS3, Line 2453: const int kLimit = rand() % (kMaxLimit + 1) + 1;
> Why did this change?
Here 'kMaxLimit' may be 0, then an exception reported like "Floating point exception".
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1508
PS3, Line 1508: void TSTabletManager::FailTabletsInDataDir(const string& uuid) {
: DataDirManager* dir_manager = nullptr;
: int uuid_idx;
: if (fs_manager_->dd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->dd_manager();
: } else if (fs_manager_->wd_manager()->FindUuidIndexByUuid(uuid, &uuid_idx)) {
: dir_manager = fs_manager_->wd_manager();
: } else {
: LOG(FATAL) << Substitute("No directory found with UUID $0", uuid);
: }
: if (dir_manager->IsDataDirFailed(uuid_idx)) {
: LOG(WARNING) << "Data directory is already marked failed.";
: return;
: }
: // Fail the directory to prevent other tablets from being placed in it.
: dir_manager->MarkDataDirFailed(uuid_idx);
: set<string> tablets = dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx);
: LOG(INFO) << Substitute("Data dir $0 has $1 tablets", uuid, tablets.size());
: for (const string& tablet_id : dir_manager->FindTabletsByDataDirUuidIdx(uuid_idx)) {
: FailTabletAndScheduleShutdown(tablet_id);
: }
: }
> Rather than reusing this exactly, I'd prefer we NOT tightly couple the hand
Here I still don't understand how to keep a single FsErrorManager. In the initialization we have two FsErrorManager for disk error, the error handle is MarkDataDirFailedByUuid. After starting we have just one FsErrorManager, the error handle is FailTabletsInDataDir. Here uuid maybe in two managers, WAL manager and data manager.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 08 Nov 2019 03:54:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#3).
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
[KUDU-2975]Spread WAL across multiple directories
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-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/fs/fs_manager.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
27 files changed, 348 insertions(+), 103 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/3
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 20:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/tserver/ts_tablet_manager.cc@1471
PS3, Line 1471: MAYBE_FAULT(FLAGS_fault_crash_after_blocks_deleted);
:
: // We do not delete the super
> I'm not sure it's safe. if a tablet has just been created, and we write two
The other way I think about it is modifying the parameters of the function Log::DeleteOnDiskData and Log::RemoveRecoveryDirIfExists.
like this:
Status Log::DeleteOnDiskData(const string& wal_dir, const string& tablet_id)
Status Log::RemoveRecoveryDirIfExists(const string& wal_dir, const string& tablet_id)
it looks safer.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 20
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Wed, 27 Nov 2019 09:11:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/3/src/kudu/fs/data_dirs.cc@723
PS3, Line 723: if (opts_.dir_type == "data") {
: // Create a per-dir thread pool.
> This style of checking makes the code more difficult to understand and main
Andrew and I discussed this at length today.
We agreed that there's both enough overlapping and non-overlapping functionality that either approach (sharing a common directory manager implementation or two different implementations) is going to require some work. Given that this patch instantiates two different DataDirManagers, I'm inclined to agree with Andrew that perhaps we should start with two completely different classes (one for managing data directories and one for managing WAL directories), and see where that takes us.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 08 Nov 2019 22:59:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has abandoned this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Abandoned
work on the branch "wal-directory". new code is https://gerrit.cloudera.org/#/c/14920/
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 25
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#25).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. Every WAL
directory has an UUID, We record the directory's UUID and all
directories's UUIDs into a file named "wal_manager_instance". We also record
the tablet's WAL directory UUID into the tablet's metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup.
Because the tablet's metadata in old version had no WAL directory, we
look for the tablet's WAL directory under all the new WAL directories.
If found, we write the talbet's WAL into WalDirManager. But flushing
metadata may causes a deadlock in the tool code, so we don't persist
the metadata immediately.
In this version, one of the WAL directorys's structure looks like this:
----wal
--------instance
--------wals
------------wal_manager_instance
------------tablet1_uuid
----------------index.0
----------------wal.0
------------tablet2_uuid
----------------index.0
----------------wal.0
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M docs/administration.adoc
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/dir_util.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
60 files changed, 3,399 insertions(+), 336 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/25
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 25
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#11).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
32 files changed, 1,413 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/11
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 11
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#22).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. Every WAL
directory has an UUID, We record the directory's UUID and all
directories's UUIDs into a file named "wal_manager_instance". We also record
the tablet's WAL directory UUID into the tablet's metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup.
Because the tablet's metadata in old version had no WAL directory, we
look for the tablet's WAL directory under all the new WAL directories.
If found, we write the talbet's WAL into WalDirManager. But flushing
metadata may causes a deadlock in the tool code, so we don't persist
the metadata immediately.
In this version, one of the WAL directorys's structure looks like this:
----wal
--------instance
--------wals
------------wal_manager_instance
------------tablet1_uuid
----------------index.0
----------------wal.0
------------tablet2_uuid
----------------index.0
----------------wal.0
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
58 files changed, 3,161 insertions(+), 322 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/22
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 22
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#15).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. We record
the tablet's WAL directory UUID into the metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup. Because the tablet's metadata in old version had
no WAL dir, we look for the tablet's WAL directory under all the new WAL
directories. If found, we write the talbet's WAL into WalDirManager. But
it's not persistent.
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory UUID form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
52 files changed, 2,785 insertions(+), 288 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/15
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 15
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
Patch Set 13:
(20 comments)
I think we could get rid of a lot of reused code from the consistency checking.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs.proto
File src/kudu/fs/fs.proto:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs.proto@58
PS13, Line 58: // A filesystem instance can contain multiple paths. One of these structures
: // is persisted in each path when the filesystem instance is created.
: message PathInstanceMetadataPB {
: // Describes this path instance as well as all of the other path instances
: // that, taken together, describe a complete set.
: required PathSetPB path_set = 1;
:
: // Textual representation of the block manager that formatted this path.
: required string block_manager_type = 2;
:
: // Block size on the filesystem where this instance was created. If the
: // instance (and its data) are ever copied to another location, the block
: // size in that location must be the same.
: required uint64 filesystem_block_size_bytes = 3;
: }
I'm not convinced we have to reuse this either, especially if we end up not using the consistency check for UUIDs. We definitely don't need the block_manager_type for WALs, and I don't think we need the block size for WALs either. We probably also don't need all_uuids if we end up not doing the consistency check.
What if the wal_manager_instance were just a simple file that had the WAL UUID in it? Something like:
message WalDirInstancePB {
optional bytes uuid = 1;
}
That way we can evolve it separately from the block_manager_instance files, while only including what we need for now.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs.proto@121
PS13, Line 121: // Tablet log is spread across a specified WAL directorie. This PB is represented
: // by the UUID of the WAL directorie it consists of.
nit reword a little bit: A tablet's WAL is placed in a single WAL directory. This directory is represented by a UUID.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.h
File src/kudu/fs/fs_manager.h:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.h@98
PS13, Line 98: // The directory root where WALs will be stored. Cannot be empty.
: std::vector<std::string> wal_roots;
Rather than keeping track of two sets of WAL roots (wal_root _and_ wal_roots), let's just use a single vector<string> wal_roots.
That way, there will be no difference between using fs_wal_dir and inputting a single directory with fs_wal_dirs.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@195
PS13, Line 195: if (!opts_.wal_root.empty() && !opts_.wal_roots.empty()) {
: return Status::IOError("Write-ahead log directory (fs_wal_dir and fs_wal_dirs)"
: " provided conflictly");
: }
nit: I think this would be nicer as a gflag validator.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@392
PS13, Line 392: if (opts_.consistency_check != ConsistencyCheckBehavior::UPDATE_ON_DISK) {
Why this check?
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@474
PS13, Line 474: report->wal_dir = canonicalized_wal_fs_roots_[0].path;
We should probably allow the FsReport to accept multiple WAL directories too.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@534
PS13, Line 534: vector<string> ancillary_dirs = JoinPathSegmentsV(
: WalDirManager::GetRootNames(canonicalized_wal_fs_roots_), kWalDirName);
How about let's have the WalDirManager manage the creation of WAL directories, like we do for the DataDirManager.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/fs_manager.cc@735
PS13, Line 735:
: string FsManager::GetTabletWalDir(const std::string& tablet_id) const {
: string dir;
: Status s = wd_manager_->FindWalDirByTabletId(tablet_id, &dir);
: if (!s.ok()) {
: LOG(WARNING) << "cannot find tablet:" << tablet_id << " wal directory." <<
: s.message().ToString();
: return "";
: }
: return JoinPathSegments(dir, tablet_id);
: }
I think we'll eventually want to have this return a Status and have a string* out-parameter. Returning special values like "" is a bit brittle.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc
File src/kudu/fs/wal_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc@302
PS13, Line 302: Status WalDirManager::UpdateInstances(
If we don't do the consistency check for WAL directories, I don't think we'll need this.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc@436
PS13, Line 436: consistency_check
I'm not convinced that we need the consistency checking behavior for WALs.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc@582
PS13, Line 582: Status WalDirManager::LoadWalDirFromPB(const std::string& tablet_id,
: const WalDirPB& pb) {
It's important to remember that we won't always have a WalDirPB, for example in cases where the tablets are from older versions of Kudu.
I know you haven't begun working on the migration path, but you should think about it before going too far, so you can make sure it makes sense.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc@604
PS13, Line 604: TryCreateWalDir
nit: maybe call this FindAndRegisterWalDir or something? That way it's clear we're looking for an existing WAL directory.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/fs/wal_dirs.cc@605
PS13, Line 605: std::lock_guard<percpu_rwlock> write_lock(dir_lock_);
We only need to hold the lock in read mode for ContainsKey(), and we don't have to hold the lock at all when we're doing the FileExists() checks. Could you rewrite this as:
{
read lock(dir_lock_);
check if dir already exists
}
// Check to see if any of our WAL directories have the given tablet ID.
set<string> wd_uuids;
for (wd : wal_dirs) {
if (WAL exists in wd) {
wd_uuids.emplace_back(wd.uuid());
}
}
// If anything is wrong (e.g. if there are no directories or multiple directories with the WAL), return an error.
if (wd_uuids.size() != 1) {
return an error
}
// Now that we know we have a WAL, insert it into the map.
{
write lock(dir_lock_);
insert wd_uuids.begin() into the map
}
return OK
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tablet/tablet_bootstrap-test.cc@121
PS13, Line 121: fs_manager_->wd_manager()->DeleteWalDir(log::kTestTablet);
Why did we have to do this?
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tablet/tablet_metadata.cc@481
PS13, Line 481: // Here cannot flush directly. May causes a deadlock in the tool code.
: // If we switch 'fs_wal_dir' to 'fs_wal_dirs', we should try to find the directory
: // where tablet's WAL is located.
: // If we found, we should record WAL directory UUID into metadata, and flush metadata.
: // otherwise maybe we should crash.
: //RETURN_NOT_OK(Flush());
Yeah, the same is true for data directories. Next time we flush metadata, we'll flush the new WAL directory UUID.
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tools/tool_action_local_replica.cc
File src/kudu/tools/tool_action_local_replica.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tools/tool_action_local_replica.cc@367
PS13, Line 367: // For new version, need load metadata, find directory from metadata.
: RETURN_NOT_OK(fs_manager.wd_manager()->CreateWalDir(tablet_id));
Won't this happen when we load the tablet metadata?
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tools/tool_action_local_replica.cc@524
PS13, Line 524: CreateWalDir
Aren't we operating on an existing tablet replica? Why are we creating a brand new WAL directory from scratch? Don't we want to use TryCreateWalDir here to find the existing WAL directory?
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_copy_client-test.cc
File src/kudu/tserver/tablet_copy_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_copy_client-test.cc@298
PS13, Line 298: fs_manager_->wd_manager()->CreateWalDir(GetTabletId());
Why are we creating a WAL directory from scratch here? Shouldn't the WAL directory already know about the tablet?
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/tserver/tablet_server-test.cc@2453
PS13, Line 2453: const int kLimit = rand() % (kMaxLimit + 1) + 1;
This isn't related to this patch, right? If not, could you put it in a separate patch so that this patch is focused on only the WAL?
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:
http://gerrit.cloudera.org:8080/#/c/14654/13/src/kudu/util/env_util.cc@362
PS13, Line 362: Status ListDirsInDir(Env* env,
: const string& path,
: vector<string>* entries) {
: RETURN_NOT_OK(env->GetChildren(path, entries));
: auto iter = entries->begin();
: while (iter != entries->end()) {
: if (*iter == "." || *iter == ".." || iter->find(kTmpInfix) != string::npos) {
: iter = entries->erase(iter);
: continue;
: }
: bool is_dir = false;
: Status s = env->IsDirectory(JoinPathSegments(path, *iter), &is_dir);
: if (!s.ok() || !is_dir) {
: iter = entries->erase(iter);
: continue;
: }
: ++iter;
: }
: return Status::OK();
: }
:
We don't need this anymore, right?
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 13
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Fri, 15 Nov 2019 07:06:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#7).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
33 files changed, 1,357 insertions(+), 87 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/7
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 7
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#2).
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
[KUDU-2975]Spread WAL across multiple directories
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-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/fs/fs_manager.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
27 files changed, 335 insertions(+), 100 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/2
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 2
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#9).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
33 files changed, 1,347 insertions(+), 89 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/9
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 9
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] [KUDU-2975]Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
YangSong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14654 )
Change subject: [KUDU-2975]Spread WAL across multiple directories
......................................................................
Patch Set 3:
I see, I will try to add a new class for managing WAL directories.
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 3
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
Gerrit-Comment-Date: Mon, 11 Nov 2019 05:45:34 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#19).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. We record
the tablet's WAL directory UUID into the metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup. Because the tablet's metadata in old version had
no WAL dir, we look for the tablet's WAL directory under all the new WAL
directories. If found, we write the talbet's WAL into WalDirManager. But
the metadata has not been persisted immediately.
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
Deleting tablet has also been modified, we put the delete WAL before the
delete metadata. When the tserver crash when deleting the WAL, the WAL
deletion succeeds but the metadata deletion fails. If the WAL is
corrupted, we will fail the tablet. If the WAL directory is empty, we
will create a new tablet(That could be a problem, maybe we should check
the data, if we find the tablet has data but WAL directory is empty, we
should fail it).
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_table-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy-test-base.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-stress-test.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
58 files changed, 3,218 insertions(+), 323 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/19
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 19
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#5).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
33 files changed, 1,358 insertions(+), 87 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/5
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 5
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#16).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new gflag named "fs_wal_dirs" to support spreading WAL across
multiple directories.
Use a new class named "WalDirManager" to manage the WAL dirs. We record
the tablet's WAL directory UUID into the metadata. We determine the
tablet WAL location based on the dir UUID recorded in metadata at
reboot.
If switch 'fs_wal_dir' to 'fs_wal_dirs', first we need use "kudu fs
update_dirs" tool to update the WAL dir. We should make sure that the
new configuration includes the old ones, otherwise some tablets may be
failed after startup. Because the tablet's metadata in old version had
no WAL dir, we look for the tablet's WAL directory under all the new WAL
directories. If found, we write the talbet's WAL into WalDirManager. But
it's not persistent.
Some WAL directories are allowed to be missing at startup. Or some disks
that hold WAL are allowed to be failed at startup. If the tablets
located on failed WAL directories, they can be recovered by master.
This modification has the following functions not completed at present:
1. WAL disk failure not support.
2. some tools about WAL not modify.
3. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory UUID form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
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/fs/fs_report.cc
M src/kudu/fs/fs_report.h
A src/kudu/fs/wal_dirs-test.cc
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/multidir_cluster-itest.cc
M src/kudu/integration-tests/open-readonly-fs-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/tablet_copy-itest.cc
M src/kudu/integration-tests/timestamp_advancement-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/mini_master-test.cc
M src/kudu/master/mini_master.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/mini_tablet_server-test.cc
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/tserver/mini_tablet_server.h
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test-base.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
53 files changed, 2,810 insertions(+), 290 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/16
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 16
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>
[kudu-CR] KUDU-2975: Spread WAL across multiple directories
Posted by "YangSong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14654
to look at the new patch set (#8).
Change subject: KUDU-2975: Spread WAL across multiple directories
......................................................................
KUDU-2975: Spread WAL across multiple directories
Add a new class for managing WAL dir. This modification has the
following functions not completed at present:
1. during switch "fs_wal_dir" to "fs_wal_dirs", not compatible.
2. WAL disk failure not support.
3. Tool about wal not modify.
4. Lack tests.
The following functions may need improvement
1. the method of selecting WAL directorie for tablet
2. delete tablet's WAL directory form metadata
Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
---
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/fs/CMakeLists.txt
M src/kudu/fs/data_dirs.cc
M src/kudu/fs/data_dirs.h
M src/kudu/fs/fs.proto
M src/kudu/fs/fs_manager-test.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
A src/kudu/fs/wal_dirs.cc
A src/kudu/fs/wal_dirs.h
M src/kudu/integration-tests/delete_tablet-itest.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.cc
M src/kudu/integration-tests/mini_cluster_fs_inspector.h
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/metadata.proto
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tserver/tablet_copy_client-test.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/path_util.h
33 files changed, 1,361 insertions(+), 90 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/14654/8
--
To view, visit http://gerrit.cloudera.org:8080/14654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ied496804421d91ff1fa63d49979fde971071506e
Gerrit-Change-Number: 14654
Gerrit-PatchSet: 8
Gerrit-Owner: YangSong <sy...@yeah.net>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: YangSong <sy...@yeah.net>