Copilot commented on code in PR #3026:
URL: https://github.com/apache/pekko/pull/3026#discussion_r3330856943
##########
cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ShardRegion.scala:
##########
@@ -320,9 +318,8 @@ object ShardRegion {
* @param stats the region stats mapping of `ShardId` to number of entities
* @param failed set of shards if any failed to respond within the timeout
*/
- @SerialVersionUID(1L) final class ShardRegionStats(val stats: Map[ShardId,
Int], val failed: Set[ShardId])
- extends ClusterShardingSerializable
- with Product {
+ @SerialVersionUID(1L) final case class ShardRegionStats(stats: Map[ShardId,
Int], failed: Set[ShardId] = Set.empty)
+ extends ClusterShardingSerializable {
Review Comment:
The PR description only covers removal of the `passivate-idle-entity-after`
backward-compat machinery, but this diff also converts `ShardRegionStats` and
`CurrentShardRegionState` from hand-written `final class … with Product` (with
custom equals/hashCode/productElement/unapply returning a single field) to a
`final case class`. That is an independent behavioral change (e.g. the
generated `unapply` now returns the 2-tuple `(stats, failed)` / `(shards,
failed)` instead of the prior single-field `Option[Map]` / `Option[Set]`, which
is a source-incompatible change for any Scala pattern match using a single
binder like `case ShardRegionStats(stats) => …`). It also reintroduces the
auto-generated public `apply`/`unapply`/`copy` on these public messages. Please
either split this into a separate PR or call it out explicitly in the
description and migration note so reviewers and downstream users can evaluate
it on its own merits.
##########
cluster-sharding/src/main/scala/org/apache/pekko/cluster/sharding/ClusterShardingSettings.scala:
##########
@@ -101,7 +101,7 @@ object ClusterShardingSettings {
val coordinatorSingletonSettings =
ClusterSingletonManagerSettings(config.getConfig("coordinator-singleton"))
- val passivationStrategySettings =
PassivationStrategySettings.fromSharding(config)
+ val passivationStrategySettings =
PassivationStrategySettings(config.getConfig("passivation"))
Review Comment:
After this change drops support for `passivate-idle-entity-after`, two
follow-ups appear to be missing from this PR:
1.
`cluster-sharding-typed/src/multi-jvm/scala/org/apache/pekko/cluster/sharding/typed/delivery/DeliveryThroughputSpec.scala`
still sets `pekko.cluster.sharding.passivate-idle-entity-after = 5s`. With
this PR, that key is silently ignored and the spec falls back to the default
120s idle timeout, which changes the spec's runtime behavior. It should be
migrated to
`pekko.cluster.sharding.passivation.default-idle-strategy.idle-entity.timeout =
5s`.
2. `cluster-sharding/src/main/resources/reference.conf` still documents
`passivate-idle-entity-after = null` under the `pekko.cluster.sharding` block.
Since the code no longer reads this key, the stanza is misleading and should be
removed (or replaced with a brief "removed; use
passivation.default-idle-strategy.idle-entity.timeout" note).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
