apache / pekko

Build highly concurrent, distributed, and resilient message-driven applications using Java/Scala
https://pekko.apache.org/
Apache License 2.0
1.17k stars 139 forks source link

handle the case where `DeviceTerminated.groupId` may not match the `DeviceGroup.groupId` #1311

Closed Ghurtchu closed 3 months ago

Ghurtchu commented 4 months ago

I was following the online tutorial on the official Pekko website: https://pekko.apache.org/docs/pekko/current/typed/guide/tutorial_4.html#:~:text=Keeping%20track%20of,%C2%B6

and I found that DeviceTerminated.groupId may not match with DeviceGroup.groupId but we don't handle such a case right now. I suggest that we keep consistency and fix it. Since such an approach is already used with RequestTrackDevice message when grId != groupId we could just add a new case in the end to log the warning about the possible scenario here: https://github.com/apache/pekko/blob/58fa510455190bd62d04f92a83c9506a7588d29c/docs/src/test/scala/typed/tutorial_4/DeviceGroup.scala#L85

case DeviceTerminated(_, grId, deviceId) =>
    context.log.warnN("Ignoring DeviceTerminated for groupId:{} and deviceId:{}. This actor is responsible for {}", grId, deviceId, groupId)
    this
laglangyue commented 4 months ago

Thanks for your report, could you submit a pr

Roiocam commented 4 months ago

DeviceTerminated only sends if the child actor is terminated, it will always match groups, i don't think there have any issue on there. Could you make a minimal reproduction?

      case trackMsg @ RequestTrackDevice(`groupId`, deviceId, replyTo) =>
        deviceIdToActor.get(deviceId) match {
          case Some(deviceActor) =>
            replyTo ! DeviceRegistered(deviceActor)
          case None =>
            context.log.info("Creating device actor for {}", trackMsg.deviceId)
            val deviceActor = context.spawn(Device(groupId, deviceId), s"device-$deviceId")
+           context.watchWith(deviceActor, DeviceTerminated(deviceActor, groupId, deviceId))
            deviceIdToActor += deviceId -> deviceActor
            replyTo ! DeviceRegistered(deviceActor)
        }
        this

      case RequestTrackDevice(gId, _, _) =>
+       context.log.warn2("Ignoring TrackDevice request for {}. This actor is responsible for {}.", gId, groupId)
        this