amin-aoulkadi opened a new issue, #1835: URL: https://github.com/apache/pekko/issues/1835
## Context Users can send a `GetShardRegionState` or `GetClusterShardingStats` message to the cluster sharding extension. Pekko then replies with a `CurrentShardRegionState` or `ClusterShardingStats` message. These replies are intended to be received and handled by users. See the full documentation at https://pekko.apache.org/docs/pekko/current/typed/cluster-sharding.html#inspecting-cluster-sharding-state. ## Issue (`CurrentShardRegionState`) The `CurrentShardRegionState` message has two fields, `shards` and `failed`: https://github.com/apache/pekko/blob/38facf752b4e7d5fb43ed7c671eb91888b41bd10/cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ShardRegion.scala#L384 `CurrentShardRegionState` is no case class, so its companion object defines an `unapply` method. However, only the `shards` field is extracted: https://github.com/apache/pekko/blob/38facf752b4e7d5fb43ed7c671eb91888b41bd10/cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ShardRegion.scala#L427-L428 This is mildly inconvenient, as it interferes with pattern matching: ```scala case CurrentShardRegionState(shards, failed) => //Error: Wrong number of argument patterns for org.apache.pekko.cluster.sharding.ShardRegion.CurrentShardRegionState; expected: (Set[org.apache.pekko.cluster.sharding.ShardRegion.ShardState]) //... Behaviors.same ``` Of course, there are alternatives that circumvent this issue (e.g. `case state: CurrentShardRegionState =>`). ## Remarks ### `ClusterShardingStats` (No Problem) The other user-facing reply, `ClusterShardingStats`, does not have this issue as it is a case class: https://github.com/apache/pekko/blob/38facf752b4e7d5fb43ed7c671eb91888b41bd10/cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ShardRegion.scala#L287 ### `ShardRegionStats` (Similar Problem) Interestingly, `ShardRegionStats` (part of the payload of `ClusterShardingStats`) exhibits the same issue - its `unapply` method does not extract the `failed` field: https://github.com/apache/pekko/blob/38facf752b4e7d5fb43ed7c671eb91888b41bd10/cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ShardRegion.scala#L320 https://github.com/apache/pekko/blob/38facf752b4e7d5fb43ed7c671eb91888b41bd10/cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ShardRegion.scala#L362-L363 ## Open Questions - Why do `CurrentShardRegionState.unapply` and `ShardRegionStats.unapply` not extract the `failed` fields? - Why are `CurrentShardRegionState` and `ShardRegionStats` normal classes instead of case classes? - Can `CurrentShardRegionState.failed` even be non-empty in practice? I could not find any usages that set the `failed` field to anything other than `Set.empty`. This is particularly intriguing because the documentation states: > If any shard queries failed, for example due to timeout if a shard was too busy to reply within the configured `pekko.cluster.sharding.shard-region-query-timeout`, `ShardRegion.CurrentShardRegionState` and `ShardRegion.ClusterShardingStats` will also include the set of shard identifiers by region that failed. <hr> The code references in this issue belong to Pekko 1.1.3, but the relevant code is at least five years old (according to Git blame). The Akka codebase has the same problem, but it has not been reported or discussed in the Akka GitHub repository (as far as I can tell). -- 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: notifications-unsubscr...@pekko.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org For additional commands, e-mail: notifications-h...@pekko.apache.org
