Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node UUID map initialized from cluster discovery #7204

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/v/cluster/bootstrap_backend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,31 @@
#include "cluster/cluster_uuid.h"
#include "cluster/commands.h"
#include "cluster/logger.h"
#include "cluster/members_manager.h"
#include "cluster/types.h"
#include "security/credential_store.h"

namespace cluster {

// node_ids_by_uuid initialization
// ===============================
// If the node UUID to ID map in `members_manager` were not initialized at
// cluster bootstrap, that would have allowed slow seed nodes (ones that miss
// their oportunity to be founding nodes because of being slow) to also possibly
// miss the assignment of `node_id` based on `seed_servers` index (as expected
// for seed nodes). That could happen if another node joins the cluster before a
// slow seed node and grabs the `node_id` that should have belonged to it.
//
// `bootstrap_backend` references `members_manager` to be able to initialize the
// node UUID to ID map in it.
Comment on lines +23 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: IMO kinda odd placement; not sure folks would know to look for this context here. Maybe move this down by L146? or somewhere in the header?


bootstrap_backend::bootstrap_backend(
ss::sharded<security::credential_store>& credentials,
ss::sharded<storage::api>& storage)
ss::sharded<storage::api>& storage,
dlex marked this conversation as resolved.
Show resolved Hide resolved
ss::sharded<members_manager>& members_manager)
: _credentials(credentials)
, _storage(storage) {}
, _storage(storage)
, _members_manager(members_manager) {}

namespace {

Expand Down Expand Up @@ -128,6 +143,10 @@ bootstrap_backend::apply(bootstrap_cluster_cmd cmd) {
}
}

// Apply initial node UUID to ID map
_members_manager.local().apply_initial_node_uuid_map(
cmd.value.node_ids_by_uuid);

// Apply cluster_uuid
co_await _storage.invoke_on_all(
[&new_cluster_uuid = cmd.value.uuid](storage::api& storage) {
Expand Down
7 changes: 6 additions & 1 deletion src/v/cluster/bootstrap_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@ namespace cluster {
* This class applies the cluster initialization message to
* - storage, persisting the cluster UUID to the kvstore,
* - credential_store, to initialize the bootstrap user
* - members_manager, to initialize node UUID map
* - TODO: apply the initial licence
*/
class bootstrap_backend final {
public:
bootstrap_backend(
ss::sharded<security::credential_store>&, ss::sharded<storage::api>&);
ss::sharded<security::credential_store>&,
ss::sharded<storage::api>&,
ss::sharded<members_manager>&);

ss::future<std::error_code> apply_update(model::record_batch);

Expand All @@ -49,8 +52,10 @@ class bootstrap_backend final {

private:
ss::future<std::error_code> apply(bootstrap_cluster_cmd);

ss::sharded<security::credential_store>& _credentials;
ss::sharded<storage::api>& _storage;
ss::sharded<members_manager>& _members_manager;
std::optional<model::cluster_uuid> _cluster_uuid_applied;
};

Expand Down
4 changes: 3 additions & 1 deletion src/v/cluster/controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ ss::future<> controller::start(cluster_discovery& discovery) {
})
.then([this] {
return _bootstrap_backend.start_single(
std::ref(_credentials), std::ref(_storage));
std::ref(_credentials),
std::ref(_storage),
std::ref(_members_manager));
})
.then([this] {
return _config_frontend.start(
Expand Down
10 changes: 10 additions & 0 deletions src/v/cluster/members_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
#include <seastar/core/shared_ptr.hh>
#include <seastar/core/smp.hh>

#include <fmt/ranges.h>

#include <chrono>
#include <system_error>
namespace cluster {
Expand Down Expand Up @@ -354,6 +356,14 @@ model::node_id members_manager::get_node_id(const model::node_uuid& node_uuid) {
return it->second;
}

void members_manager::apply_initial_node_uuid_map(uuid_map_t id_by_uuid) {
vassert(_id_by_uuid.empty(), "will not overwrite existing data");
if (!id_by_uuid.empty()) {
vlog(clusterlog.debug, "Initial node UUID map: {}", id_by_uuid);
}
_id_by_uuid = std::move(id_by_uuid);
}

template<typename Cmd>
ss::future<std::error_code> members_manager::dispatch_updates_to_cores(
model::offset update_offset, Cmd cmd) {
Expand Down
7 changes: 6 additions & 1 deletion src/v/cluster/members_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ class members_manager {
friend std::ostream& operator<<(std::ostream&, const node_update&);
};

using uuid_map_t = absl::flat_hash_map<model::node_uuid, model::node_id>;

members_manager(
consensus_ptr,
ss::sharded<controller_stm>&,
Expand Down Expand Up @@ -176,8 +178,11 @@ class members_manager {
// guarantee that the UUID has already been registered before calling.
model::node_id get_node_id(const model::node_uuid&);

// Initialize `_id_by_uuid`. Should be called once only when bootstrapping a
// cluster.
void apply_initial_node_uuid_map(uuid_map_t);

private:
using uuid_map_t = absl::flat_hash_map<model::node_uuid, model::node_id>;
using seed_iterator = std::vector<config::seed_server>::const_iterator;
// Cluster join
void join_raft0();
Expand Down