feat(pubsub/pstest): subscription message ordering#6257
feat(pubsub/pstest): subscription message ordering#6257hongalex merged 3 commits intogoogleapis:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
hongalex
added
the
kokoro:run
kokoro-team
removed
the
kokoro:run
| orderingKey = id | ||
| } | ||
| if val, ok := orderingKeyMap[orderingKey]; !ok || m.proto.Message.PublishTime.AsTime().Before(val.m.proto.Message.PublishTime.AsTime()) { | ||
| orderingKeyMap[orderingKey] = msg{m: m, id: id} |
There was a problem hiding this comment.
I'm a little confused as to how this works. If there are multiple messages with the same ordering key, doesn't this just override the map at that ordering key? Or is this intention only to deliver 1 message per ordering key at a time?
There was a problem hiding this comment.
Yes, it was intended. Alternative option would be to sort the collection accordingly but I'm not sure if that would work properly with multiple clients reusing the same subscription (AFAIU it's a valid use case and messages with the same ordering key should still not be processed in parallel) so I went ahead with a safer option as my first choice
There was a problem hiding this comment.
I'm assuming that real pubsub just makes sure that any consecutive messages with the same ordering key get delivered to the same client until all the messages with that key get acknowledged/deleted by retention (I actually want to test it out now) but I feel like replicating that behavior here would require a lot more changes and may not be suitable for a testing library.
One of the downsides of this particular approach is that it would be harder for you to ensure that client library correctly handles the case where multiple messages with the same ordering key get delivered to a client but since there are official client libraries in other languages I'm assuming that testing process for those are a lot more robust
There was a problem hiding this comment.
Regardless of that I'd be happy to change the approach if you think something else works better
There was a problem hiding this comment.
I'm assuming that real pubsub just makes sure that any consecutive messages with the same ordering key get delivered to the same client until all the messages with that key get acknowledged/deleted by retention
Yes, there is subscriber affinity in Pub/Sub with ordering keys but also agree this is probably out of the scope of the fake. I also think that testing subscriber affinity is something that is out of scope for testing in the client library so I think this approach works well. Thanks for authoring this change!
kokoro-team
removed
the
kokoro:run
3 participants
Fix #6255
It isn't the most effective solution (something like storing
map[orderingKey || id]struct{id, *message}would be more effective) but considering that the package is intended to use for unit tests I think that lower risk outweighs the performance penalty. I would be happy to update it to a more effective solution if needed