search

devsisters / shardcake

Sharding and location transparency for Scala
https://devsisters.github.io/shardcake/
Apache License 2.0
389 stars 30 forks source link

Fix streaming response interruption #77

Closed mleclercq closed 1 year ago

mleclercq commented 1 year ago

Unfortunately, there were a couple of issues that slipped through in https://github.com/devsisters/shardcake/pull/53

  1. A change in zio-grpc made the shutdown of a pod to hang when there are active streaming responses. AFAIU the change in zio-grpc causing this issue is https://github.com/scalapb/zio-grpc/commit/6bbdcccb7838d0fb5c9b41685a6b55e652144bd3 . That change was not on the original "streaming" branch on which I did all my testing and was only bring in when the main branch was merged in. The solution is to make sure really all exit values from the response stream (including failures because of interruption) are pushed in the reply queue. Apparently catchAllCause is not enough which surprises me...
  2. The second issue is that it is possible for the expirationFiber of an entity to be uninterruptible blocking the delivery of subsequent message (because we interrupt that fiber before delivering a message). This may happen when the messenger.sendStream is called in an uninterruptible section and the entity is local so the whole message delivery happens in that uninterruptible section creating an uninterruptible expirationFiber. I noticed that the implementation of Messenger.send effect has a .interruptible while sendStream doesn't:

https://github.com/devsisters/shardcake/blob/762c95f6d47b2dc8f1364aeebff4f6cb2c97a29a/entities/src/main/scala/com/devsisters/shardcake/Sharding.scala#L267-L281 I'm not exactly sure what's the intention of that .interruptible in Messenger.send and if it should be added to sendStream, but in this PR I'm adding the .interruptible on the expirationFiber itself so whatever happen this is always interruptible