Only rate limit backfills by nickvikeras · Pull Request #27735 · MystenLabs/sui

nickvikeras · 2026-08-18T00:57:55Z

Description

We have had a few blips where a read load spike causes the write rate limits to kick in while the cluster autoscales which leads to lag. This PR lets writes for pipelines who are ahead of the "tip of chain" bypass flow control. "Tip of chain" being defined as the latest checkpoint known to the ingestion client at indexer startup. These tip-of-chain writes are already paced by chain execution, and the additional load on the DB by simply keeping up with that pace in minimal. Plus most clients would rather accept some high percentile read latency for fresher data during these load spikes than make the opposite trade.

Test plan

Given the dependency on real workloads and bigtable behavior, the easiest way to test it for real is to deploy it. We should stop seeing lag spikes during minor load spikes in prod when this is deployed.

vercel · 2026-08-18T00:58:02Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
sui-docs	Ready	Preview	Aug 20, 2026 2:31am

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
multisig-toolkit	Ignored	Preview	Aug 20, 2026 2:31am
sui-kiosk	Ignored	Preview	Aug 20, 2026 2:31am

tpham-mysten

Looks reasonable. Having a comment on if we can simplify 2 fields for client in BigTableClient

tpham-mysten · 2026-08-19T16:15:19Z

+        self.shards.clone()
+    }
+
+    pub(crate) fn requires_batch_write_flow_control(&self) -> bool {


nit: is_batch_write_flow_control_required? [or something similar to that]

As the other look like an action?

tpham-mysten · 2026-08-19T16:45:16Z

 pub struct BigTableClient {
    table_prefix: String,
    client: BigtableInternalClient<AuthChannel<ChannelPool>>,
+    flow_control_disabled_client: BigtableInternalClient<AuthChannel<ChannelPool>>,


Could we get away with a single client here and handle the enable/disable decision at the call site instead? Two clients for one boolean feels heavier than it needs to be.

amnn

LGTM -- I think there might be a way to avoid threading this through each handler though (suggested in comments).

amnn · 2026-08-19T16:32:59Z

-    use std::sync::Mutex;
    use std::time::Instant;

+    use parking_lot::Mutex;


out of interest, why the change in Mutex impl?

The LLM just decided to do it because the repo guidance says to prefer parking_lot over std::sync for cases where you are just immediately calling .unwrap(). It's kind of a distraction but it seemed good to do if that's what the repo guidance is 🤷‍♂️

Sure why not 😅 I have yet to come across a scenario where I wouldn't unwrap the poison guard...

^This is what the LLM told me when I asked it the same question, and I took it's answer for granted. But I don't actually seeing parking_lot referenced in the monorepo Claude.md file 🤔

Apparently it came from omp's harness-level rust rules - https://github.com/can1357/oh-my-pi/blob/main/packages/coding-agent/src/discovery/builtin-rules/rs-parking-lot.md

I had assumed it came from Sui's Claude.md file, which is why I just let the agent do it initially.

amnn · 2026-08-19T16:39:59Z

            registry,
        )
        .await?;
+        let latest_checkpoint_at_startup = indexer.latest_checkpoint_at_startup();


What I'd had in mind:

Initialise the IngestionClient from ClientArgs, then you can query the latest checkpoint agains that without introducing a new API.

This also means you can parametrize or decorate the store with this information to centralize rate limiting control there, rather than pass latest_checkpoint_at_startup into each pipeline's handler. The contract is just that the pipelines pass checkpointed records to the store to commit.

Then you construct a new indexer instance with Indexer::with_ingestion_clients.

I may be missing a detail why that's not possible, but just offering in case that cleans things up.

This worked.

amnn · 2026-08-19T16:42:24Z

 #[derive(Clone)]
 pub struct BigTableStore {
    client: BigTableClient,
+    batch_writer: BigTableBatchWriter,


How come there's a client sitting outside batch_writer as well as the one inside? Would it be possible to inline live_client and backfill_client into the store and connection types (with the existing outer client being the live_client?) Hard for me to tell whether BigTableBatchWriter is justifying itself.

wlmyng

The bool name use_batch_write_flow_control describes the consequence, but the thing being propagated through the layers is a fact on whether the write was triggered by a pre-startup-checkpoint commit. The
policy of backfill -> flow-controlled client is applied at the very bottom in the router. It might be worth representing this as a {Commit/Write}Source { Backfill, Live } with the OR/any() logic, which helps elucidate
locations like write_entries(table, entries, Source::Backfill)

wlmyng · 2026-08-19T16:24:29Z

+/// A BigTable entry paired with the checkpoint that produced it.
+pub struct CheckpointedEntry {
+    checkpoint: u64,
+    entry: Entry,
+}
+
+impl CheckpointedEntry {
+    fn new(entry: Entry, checkpoint: u64) -> Self {
+        Self { checkpoint, entry }
+    }


it seems that the BitmapHandler side already tracks some max_cp to stamp watermarks on the merged output. Might be worth changing the framework so batch() receives the checkpoint as a parameter, since the batch() call is made per-checkpoint.

I was trying to avoid a breaking framework change since to fix these lag spikes, but I agree it would be nice to have the checkpoint number in batch().

wlmyng · 2026-08-19T16:28:38Z

+    async fn write_entries(
+        &self,
+        table: &str,
+        entries: impl IntoIterator<Item = Entry>,
+        use_batch_write_flow_control: bool,
+    ) -> Result<()> {
+        let mut client = if use_batch_write_flow_control {
+            self.backfill_client.clone()
+        } else {
+            self.live_client.clone()
+        };
+        client.write_entries(table, entries).await
+    }
+}


i see, the BigtableHandler threads the use_batch_write_flow_control all the way through here, where we give the backfill_client or live_client aka client.without_batch_write_flow_control()

So it's the header that informs the BigTable server whether the client cooperates with rate limiting

wlmyng · 2026-08-19T16:53:13Z

+        let use_batch_write_flow_control = chunk
+            .rows
+            .iter()
+            .any(|row| row.use_batch_write_flow_control);


took me a sec to figure this out, am i understanding correctly that:

the bitmap/handler.rs receives checkpoints from framework, which gets converted into batched bitmap values stamped with backfill or live

on handoff to committer, actually the batch is sharded up for parallelism, and shard workers handle things from here

each shard worker merges the shard into an accumulation, and for any changes, emits a row. Consists of the label, backfill or live, and the actual contents, which is the whole bitmap

in the writer.rs writer task, regroup rows into chunks and send one. Here, one backfill row pollutes the rest

## Summary Cherry-pick of #27735 to the 1.78 release branch. Original commit: bfdabd3

## Summary Cherry-pick of #27735 to the 1.77 release branch. Original commit: bfdabd3

Only rate limit backfills

f13af19

nickvikeras requested a review from a team as a code owner August 18, 2026 00:57

nickvikeras requested review from amnn, evan-wall-mysten, tpham-mysten and wlmyng August 18, 2026 00:57

nickvikeras deployed to sui-typescript-aws-kms-test-env August 18, 2026 00:57 — with GitHub Actions Active

vercel Bot deployed to Preview – sui-docs August 18, 2026 01:01 View deployment

nickvikeras requested a review from bmwill August 18, 2026 13:37

tpham-mysten approved these changes Aug 19, 2026

View reviewed changes

amnn approved these changes Aug 19, 2026

View reviewed changes

wlmyng reviewed Aug 19, 2026

View reviewed changes

remove dead method

10c79e5

nickvikeras deployed to sui-typescript-aws-kms-test-env August 19, 2026 17:32 — with GitHub Actions Active

vercel Bot deployed to Preview – sui-docs August 19, 2026 17:34 View deployment

refactor based on PR comments

e3f98f8

nickvikeras deployed to sui-typescript-aws-kms-test-env August 20, 2026 02:28 — with GitHub Actions Active

vercel Bot deployed to Preview – sui-docs August 20, 2026 02:31 View deployment

nickvikeras merged commit bfdabd3 into main Aug 20, 2026
63 checks passed

nickvikeras deleted the nickv/cache-at-startup branch August 20, 2026 13:43

This was referenced Aug 20, 2026

[1.77] Only rate limit backfills (#27735) #27769

Merged

[1.78] Only rate limit backfills (#27735) #27770

Merged

nickvikeras added a commit that referenced this pull request Aug 20, 2026

[1.78] Only rate limit backfills (#27735) (#27770)

ea19d79

## Summary Cherry-pick of #27735 to the 1.78 release branch. Original commit: bfdabd3

nickvikeras added a commit that referenced this pull request Aug 20, 2026

[1.77] Only rate limit backfills (#27735) (#27769)

7306046

## Summary Cherry-pick of #27735 to the 1.77 release branch. Original commit: bfdabd3

Uh oh!

Conversation

nickvikeras commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test plan

Uh oh!

vercel Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tpham-mysten left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amnn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wlmyng left a comment

Choose a reason for hiding this comment

Uh oh!

wlmyng Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nickvikeras commented Aug 18, 2026 •

edited

Loading

vercel Bot commented Aug 18, 2026 •

edited

Loading

wlmyng Aug 19, 2026 •

edited

Loading