Skip to content

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jul 14, 2025

Related

What

Makes the default flush tick 200ms for file & memory sinks and leaves it at 8ms for grpc sinks.

To facilitate this, I introduced per sink batcher config defaults and had to make the batcher configuration updatable (since Python always first creates a recording stream with a memory sink and then patches it in later). Unfortunately not all config optionscan be updated after the fact, but I think the solution here is good enough for what is practically needed.

@Wumpf Wumpf added include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Jul 14, 2025
Copy link

github-actions bot commented Jul 14, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
1d81039 https://rerun.io/viewer/pr/10620 +nightly +main

Note: This comment is updated whenever you push a commit.

Copy link

github-actions bot commented Jul 14, 2025

Latest documentation preview deployed successfully.

Result Commit Link
1d81039 https://landing-a0440xpxt-rerun.vercel.app/docs

Note: This comment is updated whenever you push a commit.

@teh-cmc teh-cmc self-requested a review July 15, 2025 07:26
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

Neat.

That's both pretty tricky and very sensitive to regressions though, a few tests would be welcome:

  • Make sure setting the env does override the per-sink logic and vice-versa
  • Make sure updating the sink during logging does update the conf properly etc

@Wumpf Wumpf added the do-not-merge Do not merge this PR label Jul 15, 2025
@Wumpf Wumpf removed the do-not-merge Do not merge this PR label Jul 15, 2025
@Wumpf Wumpf force-pushed the andreas/sink-dependent-default-flush-settings branch from b571e5f to ab3095e Compare July 15, 2025 10:02
@Wumpf Wumpf added the do-not-merge Do not merge this PR label Jul 15, 2025
@Wumpf
Copy link
Member Author

Wumpf commented Jul 15, 2025

@rerun-bot full-check

Copy link

@Wumpf Wumpf merged commit 936f980 into main Jul 15, 2025
82 of 85 checks passed
@Wumpf Wumpf deleted the andreas/sink-dependent-default-flush-settings branch July 15, 2025 13:11
teh-cmc added a commit that referenced this pull request Jul 16, 2025
)

### What
We changed the behavior of the batcher in:
#10620

However, the only way to override this behavior was to set environment
variables, which can be tricky/annoying in some circumstances. This
plumbs through the configuration settings so that this can be specified
explicitly now.

---------

Co-authored-by: Clement Rey <[email protected]>
emilk added a commit that referenced this pull request Aug 6, 2025
### Related
* Introduced in #10620

### What
Environment variables are annoying
emilk added a commit that referenced this pull request Aug 6, 2025
### Related
* Introduced in #10620

### What
Environment variables are annoying
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Do not merge this PR include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust default micro-batcher settings
2 participants