oops, this should be layout.normal.hash_type

thanks for catching this!

layout.normal.hash_type = rgw::BucketHashType::Mod

The variable was removed, but the comments immediately before describing the variable were left behind, making reading the code confusing.

ack. will address in the currently open pr 35175

All these transformations inserting layout.current_index.layout.normal throughout makes the code harder to understand. It was already super-repetetive, but this makes it stand out even more. I therefore wonder if it should just be a function on bucket_info or bucket_info.layout -- a function that returns a value 1 or larger to indicate the actual # of shards.

this one too.

Thank you, @smanjara!

thanks @ivancich. yeah, there's a bunch of new indirection happening here. part of it comes from the need to track a separate current_index layout and target_index during reshard. another part comes from the BucketIndexType abstraction, where only certain index types actually have a shard count - Indexless buckets would be an example of a type that doesn't, and Matt has also been exploring a tree-like index

so my long-term goal is to factor out most of the places where common code relies on a num_shards, so that they're constrained to code paths that a) have already checked the BucketIndexType, and b) are operating on a specific layout type like bucket_index_normal_layout& layout and have easy access to layout.num_shards

in the short-term, i do think a helper function is a good idea to cut down on the boilerplate. in later refactoring work, we can just grep for that function name to identify the places that we're making these unchecked assumptions about the index type. instead of a member function though, i'd recommend a free function like this:

// rgw_bucket_layout.h
inline uint32_t current_num_shards(const BucketLayout& layout) {
  return std::max(layout.current_index.layout.normal.num_shards, 1u);
}

Yeah, this looks great! The true problem preceded these changes and has been around for a while, which is that the same logic was distributed (copy/pasted) throughout the code when it should have been centralized. The refactoring and its added indirection only made this issue pop out.

Thanks, @smanjara and @cbodley!

helper function added in #35175