Prevent duplicate arguments from getting into a single collapser RequestBatch

[mattrjacobs]

If this is attempted, then 1 of 2 things can occur:
If request-caching is on: the response for the 2nd-nth instance of the argument is the same as the first
If off: the response for the 2nd-nth instance of the argument is an error

This fixes the failing unit test supplied in #1176

[edudar]

Doesn't this, kind of, break collapser with global context? We don't use request context (because we don't have multiple requests for the same data within one request) and we don't use request caching because of this. At the same time, our service takes an input that is user-generated and may (and will) contain duplicates. As we want to minimize the impact on services down the flow we use global context for collapser here and that's where the issue appear since now a whole ton of errors fall through...

[mattrjacobs]

@edudar Thanks for the report, this case didn't come to mind during the fix above but seems like a valid one. We actually ended up not using global-collapsing anywhere internally, as our use of request-level state makes it impossible.

Any suggestions on how this might be fixed for your case? What happens if you turn request-caching on?

[anton-antonov]

Thanks @mattrjacobs
Request-caching does nothing actually.
Incoming load looks like enormous amount of request to the single resource (much simplified), thus, caching on a request level creates cache with single item within, and it is disposed when response is sent to the user.
We collapse all similar requests and produce a single call to downstream service instead of tons of them.

Other use-case is the counter service (update service) when request cashing can't be enabled from the logic perspective. It counts amount of similar requests. And we want to increment persistent counter one time with +100 instead of 100 times with +1. Request caching in this case produces only single +1 operation for all 100 duplicates during collapse interval.

Looks like the easy check for the collapser scope before throwing an exception should solve the issue. Global scope may and will have duplicated requests and there is no way to enable request caching. Request-scopes, on the other hand, will and probably should use caching, thus, it may be natural to not allow duplicates.

My opinion, is that the logic for handling duplicates should be in a final collapser implementation and it should decide what to do with duplicates, ignore, throw an error or aggregate them in a custom way, etc.