Re: [PR] SOLR-12831: Clean up ZK nodes after shard deletion is invoked [solr]
psalagnac merged PR #3314: URL: https://github.com/apache/solr/pull/3314 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-12831: Clean up ZK nodes after shard deletion is invoked [solr]
dsmiley commented on code in PR #3314: URL: https://github.com/apache/solr/pull/3314#discussion_r2035989813 ## solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java: ## @@ -125,4 +131,19 @@ private void setSliceState(String collectionName, String shardId, Slice.State st collectionName, c -> c.getSlice(shardId).getState() == state); } + + /** Check whether shard metadata exist in Zookeeper. */ + private void assertShardMetadata(String collection, String sliceId, boolean shouldExist) Review Comment: could this be improved to scan the ZK nodes for any ZNode that matches the shard ID? This way, new/unexpected stuff could be detected. ## solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteShardCmd.java: ## @@ -238,4 +239,33 @@ private List getReplicasForSlice(String collectionName, Slice slice } return sourceReplicas; } + + /** + * Best effort to delete Zookeeper nodes that stored other details than the shard itself in + * cluster state. If we fail for any reason, we just log and the shard is still deleted. + */ + private void cleanupZooKeeperShardMetadata(String collection, String sliceId) Review Comment: Ah; I forgot what the layout was. Yes; this is the best we can do. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-12831: Clean up ZK nodes after shard deletion is invoked [solr]
psalagnac commented on code in PR #3314: URL: https://github.com/apache/solr/pull/3314#discussion_r2035840059 ## solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteShardCmd.java: ## @@ -238,4 +239,33 @@ private List getReplicasForSlice(String collectionName, Slice slice } return sourceReplicas; } + + /** + * Best effort to delete Zookeeper nodes that stored other details than the shard itself in + * cluster state. If we fail for any reason, we just log and the shard is still deleted. + */ + private void cleanupZooKeeperShardMetadata(String collection, String sliceId) Review Comment: Not sure what you mean by _everything_. Unfortunately, we don't have a per shard root in Zookeeper. I wish we had, it should simpler to remove all info specific to a shard, but the transition to a different data layout will be ever complex. The naive approach is to remove an hardcoded list of ZK node. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-12831: Clean up ZK nodes after shard deletion is invoked [solr]
dsmiley commented on code in PR #3314: URL: https://github.com/apache/solr/pull/3314#discussion_r2034228946 ## solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteShardCmd.java: ## @@ -238,4 +239,33 @@ private List getReplicasForSlice(String collectionName, Slice slice } return sourceReplicas; } + + /** + * Best effort to delete Zookeeper nodes that stored other details than the shard itself in + * cluster state. If we fail for any reason, we just log and the shard is still deleted. + */ + private void cleanupZooKeeperShardMetadata(String collection, String sliceId) Review Comment: This approach looks for very specific things, and thus we need to maintain it and may miss things. Don't we want to delete everything? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
