Skip to content

Conversation

@timmartin-stripe
Copy link
Contributor

Context

We've found that these buffers are very important tuning parameters. Additionally, they are very stage specific. Sometimes, you need want small buffers and other times large buffers depending on the performance characteristics of the stage. Setting them all via the default RX configuration is not a strong strategy. Especially because that needs to be set when mantis-agent spins up; not necessarily when the job spins up.

This commit is mostly plumbing to get the buffer size to the stage executors.

Checklist

  • ./gradlew build compiles code correctly
  • Added new tests where applicable
  • ./gradlew test passes all tests
  • Extended README or added javadocs where applicable

We've found that these buffers are very important tuning parameters.
Additionally, they are very stage specific.  Sometimes, you need want
small buffers and other times large buffers depending on the performance
characteristics of the stage.  Setting them all via the default RX
configuration is not a strong strategy.  Especially because that needs
to be set when mantis-agent spins up; not necessarily when the job spins
up.

This commit is mostly plumbing to get the buffer size to the stage
executors.
.map(observable -> c
.call(context, observable
.observeOn(Schedulers.computation())
.observeOn(Schedulers.computation(), bufferSize)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key update. The default will be the same rx.internal.util.RxRingBuffer.SIZE

@github-actions
Copy link

Test Results

661 tests  ±0   650 ✅ ±0   9m 4s ⏱️ -2s
152 suites ±0    11 💤 ±0 
152 files   ±0     0 ❌ ±0 

Results for commit 3254f91. ± Comparison against base commit 3ba5b5f.

// copy reference, then apply request specific filters, sampling
Observable<T> requestObservable = observableToServe;

// decouple the observable on a separate thread and add backpressure handling
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never used. Figured it was good to clean up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants