Enhance RestMulti, configurable demand + distinct objects #38801
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@geoand there it is. Haven't added tests yet (hence it's still a draft-PR). |
snazy
commented
Feb 15, 2024
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| private static class ChunkedStreamingMultiSubscriber extends StreamingMultiSubscriber { |
There was a problem hiding this comment.
Removed this inner class, because it's doing what StreamingMultiSubscriber does with encodeAsJsonArray==false
|
Nice! |
snazy
force-pushed
the
rest-multi-enhancements
branch
from
6db9089 to
b2dd31d
Compare
|
Added test cases and updated docs |
snazy
force-pushed
the
rest-multi-enhancements
branch
from
b2dd31d to
1cfe6f5
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🙈 The PR is closed and the preview is expired. |
Adds two enhancements:
1. Produce multiple JSON objects, not just an array
2. Make requested demand configurable
== Produce multiple JSON objects
Currently, `PublisherResponseHandler.StreamingMultiSubscriber` produces a JSON array, where each emitted item is encoded as a JSON array element. For some use cases it is easier to consume a bunch of "bare" JSON objects - i.e. just write the individual JSON objects, possibly separated by a newline. As an option, of course.
Proposal to add:
```java
RestMulti.fromMultiData(multi).encodeAsArray(false)...
```
With `encodeAsArray(false)`, the produced JSON would look like this:
```json
{"some": "value"}
{"some": "value"}
{"some": "value"}
```
`encodeAsArray(true)` or omitting it would use the current behavior and produce something like this:
```json
[{"some": "value"},
{"some": "value"},
{"some": "value"}
}
```
== Configure request-demand
All implementations of `PublisherResponseHandler.AbstractMultiSubscriber` work with a hard-coded request-demand of `1`, which means that every emitted item is "produced"/"computed" serially / one-after-the-other. If the computation of individual items takes somewhat longer, possibly waiting for remote resources to reply, it makes sense to use a higher demand to produce multiple items concurrently.
For example, if each item takes maybe 250 ms (requesting data from a remote source) to be produced, and 100 items are produced, it currently takes 25 seconds. With a higher concurrency it would take a fraction of that time. I.e. if the use case is known to be not CPU but (async) I/O bound, it _might_ be legit/feasible to use a high demand.
Proposal to add:
```
RestMulti.fromMultiData(multi).withDemand( (long) 123 )...
```
Which would pass `123` as the demand for all call sites to `Subscription.request` in implementations of `PublisherResponseHandler.AbstractMultiSubscriber`.
snazy
force-pushed
the
rest-multi-enhancements
branch
from
1cfe6f5 to
0b1bf99
Compare
Status for workflow
|
Status for workflow
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds two enhancements:
Produce multiple JSON objects
Currently,
PublisherResponseHandler.StreamingMultiSubscriberproduces a JSON array, where each emitted item is encoded as a JSON array element. For some use cases it is easier to consume a bunch of "bare" JSON objects - i.e. just write the individual JSON objects, possibly separated by a newline. As an option, of course.Proposal to add:
With
encodeAsArray(false), the produced JSON would look like this:{"some": "value"} {"some": "value"} {"some": "value"}encodeAsArray(true)or omitting it would use the current behavior and produce something like this:[ {"some": "value"}, {"some": "value"}, {"some": "value"} ]Configure request-demand
All implementations of
PublisherResponseHandler.AbstractMultiSubscriberwork with a hard-coded request-demand of1, which means that every emitted item is "produced"/"computed" serially / one-after-the-other. If the computation of individual items takes somewhat longer, possibly waiting for remote resources to reply, it makes sense to use a higher demand to produce multiple items concurrently.For example, if each item takes maybe 250 ms (requesting data from a remote source) to be produced, and 100 items are produced, it currently takes 25 seconds. With a higher concurrency it would take a fraction of that time. I.e. if the use case is known to be not CPU but (async) I/O bound, it might be legit/feasible to use a high demand.
Proposal to add:
Which would pass
123as the demand for all call sites toSubscription.requestin implementations ofPublisherResponseHandler.AbstractMultiSubscriber.