David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1952 Remove round-robin for block placement ......................................................................
Patch Set 35: (5 comments) http://gerrit.cloudera.org:8080/#/c/6636/35/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: PS35, Line 477: Status DataDirManager::GetNextDataDir(const CreateBlockOptions& opts, DataDir** dir) { : // Lock shared to not block other reads to group_by_tablet_map_. : shared_lock<rw_spinlock> lock(dir_group_lock_.get_lock()); : const vector<uint16_t>* group_uuid_indices; : vector<uint16_t> all_uuid_indices; : if (PREDICT_FALSE(opts.tablet_id.empty())) { : // This should only be reached by some tests; in cases where there is no : // natural tablet_id, select a data dir randomly. : DCHECK(IsGTest()); : AppendKeysFromMap(data_dir_by_uuid_idx_, &all_uuid_indices); : group_uuid_indices = &all_uuid_indices; : } else { : // Select the data dir group for the tablet. : DataDirGroup* group = FindOrNull(group_by_tablet_map_, opts.tablet_id); : if (group == nullptr) { : return Status::NotFound("DataDirGroup not found for tablet", opts.tablet_id); : } : group_uuid_indices = &group->uuid_indices(); : } : vector<int> random_indices(group_uuid_indices->size()); : iota(random_indices.begin(), random_indices.end(), 0); : shuffle(random_indices.begin(), random_indices.end(), default_random_engine(rng_.Next())); : : // Randomly select a member of the group that is not full. : for (int i : random_indices) { : uint16_t uuid_idx = (*group_uuid_indices)[i]; : DataDir* candidate = FindOrDie(data_dir_by_uuid_idx_, uuid_idx); : RETURN_NOT_OK(candidate->RefreshIsFull( : DataDir::RefreshMode::EXPIRED_ONLY)); : if (!candidate->is_full()) { : *dir = candidate; : return Status::OK(); : } : } : return Status::IOError( : "All data directories are full. Please free some disk space or " : "consider changing the fs_data_dirs_reserved_bytes configuration " : "parameter", "", ENOSPC); : } I guess I'm missing context, but what is this for? http://gerrit.cloudera.org:8080/#/c/6636/35/src/kudu/fs/fs.proto File src/kudu/fs/fs.proto: PS35, Line 114: are nit: I know that whether data is plural or singular a hot topic among grammar nerds, but IIRC we mostly use the singular http://gerrit.cloudera.org:8080/#/c/6636/35/src/kudu/fs/log_block_manager.h File src/kudu/fs/log_block_manager.h: PS35, Line 104: the remove http://gerrit.cloudera.org:8080/#/c/6636/35/src/kudu/tablet/metadata.proto File src/kudu/tablet/metadata.proto: PS35, Line 127: are :) PS35, Line 129: from a previous version of Kudu mention a specific version -- To view, visit http://gerrit.cloudera.org:8080/6636 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9828147f4fa5c4d7f6ed23441dca5a116b8cb11b Gerrit-PatchSet: 35 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
