diff --git a/lib/collection/src/shards/replica_set/mod.rs b/lib/collection/src/shards/replica_set/mod.rs index f4046aea4a..ebeb1d8cfd 100644 --- a/lib/collection/src/shards/replica_set/mod.rs +++ b/lib/collection/src/shards/replica_set/mod.rs @@ -1080,6 +1080,18 @@ impl ShardReplicaSet { is_readable && !self.is_locally_disabled(peer_id) } + /// Check if this shard is active. + /// By active, we mean, that at least one replica have `is_active` state. + /// It is possible, that some replicas are not active, if they are created in a `Partial` state. + /// For example, during tenant promotion. + pub fn shard_is_active(&self) -> bool { + let replica_state = self.replica_state.read(); + replica_state + .peers() + .values() + .any(|state| state.is_active()) + } + /// Check if this peer can be used as a source of truth within a shard_id. /// For instance: /// - It can be the only receiver of updates diff --git a/lib/collection/src/shards/shard_holder/mod.rs b/lib/collection/src/shards/shard_holder/mod.rs index d759191f7f..3c64054362 100644 --- a/lib/collection/src/shards/shard_holder/mod.rs +++ b/lib/collection/src/shards/shard_holder/mod.rs @@ -585,6 +585,11 @@ impl ShardHolder { debug_assert!(false, "Do not expect empty shard selector") } ShardSelectorInternal::All => { + let is_custom_sharding = match self.sharding_method { + ShardingMethod::Auto => false, + ShardingMethod::Custom => true, + }; + for (&shard_id, shard) in self.shards.iter() { // Ignore a new resharding shard until it completed point migration // The shard will be marked as active at the end of the migration stage @@ -598,6 +603,15 @@ impl ShardHolder { continue; } + // Technically, we could skip inactive shards regardless of sharding method, + // as we do not expect that shard id can even become inactive on all replicas. + // (if it happens, means there is a bug) + // But for earlier detection of such issues, we only do this check for custom sharding, + // where this situation is expected for the case of tenant promotion. + if is_custom_sharding && !shard.shard_is_active() { + continue; + } + let shard_key = self.shard_id_to_key_mapping.get(&shard_id); res.push((shard, shard_key)); } diff --git a/tests/consensus_tests/test_tenant_promotion.py b/tests/consensus_tests/test_tenant_promotion.py index 20070920a2..9389b8d1c3 100644 --- a/tests/consensus_tests/test_tenant_promotion.py +++ b/tests/consensus_tests/test_tenant_promotion.py @@ -26,6 +26,21 @@ def match_results(results1: List[dict], results2: List[dict]): return True, "" +def scroll_points(peer_url, shard_key, limit=10): + resp = requests.post( + f"{peer_url}/collections/{COLLECTION_NAME}/points/scroll", + json={ + "limit": limit, + "with_payload": True, + "with_vector": False, + "shard_key": shard_key, + } + ) + assert_http_ok(resp) + + results = resp.json()["result"]["points"] + return results + def search_points(peer_url): query_vector = [0.1, 0.2, 0.3, 0.4] resp = requests.post( @@ -118,6 +133,12 @@ def test_tenant_promotion_simple(tmp_path: pathlib.Path): match, msg = match_results(results, result2) assert match, f"Results differ after tenant1 shard creation: {msg}" + # Check that global read requests work + _scroll = scroll_points(peer_api_uris[0], shard_key=None, limit=10) + + # Check that collection info is still available + _info = get_collection_info(peer_api_uris[0], COLLECTION_NAME) + n_points = 100 upsert_random_points( peer_api_uris[0],