Skip to content

sync: replace hardcoded concurrency limit with --concurrency flag#5196

Closed
luketchang wants to merge 1 commit into
databricks:mainfrom
luketchang:feat/sync-concurrency-flag
Closed

sync: replace hardcoded concurrency limit with --concurrency flag#5196
luketchang wants to merge 1 commit into
databricks:mainfrom
luketchang:feat/sync-concurrency-flag

Conversation

@luketchang

Copy link
Copy Markdown

The sync engine capped in-flight requests at a hardcoded MaxRequestsInFlight = 20, applied to file PUTs, deletes, mkdirs and rmdirs alike. That ceiling is fine for most workspaces but is too conservative for fast networks / high-tier workspaces (slow uploads of larger app bundles) and too aggressive when the workspace is rate-limiting (we want to dial it down without recompiling).

This change:

  • Adds Concurrency to libs/sync.SyncOptions, normalised to MaxRequestsInFlight in sync.New when unset so existing callers don't need updating.
  • Threads the limit through groupRunParallel/applyDiff in libs/sync/watchdog.go; the constant is kept as the documented default.
  • Surfaces a --concurrency int flag on the two user-facing entry points that drive this code path: 'databricks sync' and 'databricks bundle sync' (the latter is what backs the apps deploy/sync flow).
  • Validates --concurrency >= 1 at flag-parse time per the project rule on rejecting incompatible inputs early.
  • Adds TestGroupRunParallelHonorsLimit to lock in the parallelism cap.

Slack thread:
https://replit.slack.com/archives/C0A2Z9042FR/p1778082184841149

Changes

Why

Tests

The sync engine capped in-flight requests at a hardcoded
MaxRequestsInFlight = 20, applied to file PUTs, deletes, mkdirs and
rmdirs alike. That ceiling is fine for most workspaces but is too
conservative for fast networks / high-tier workspaces (slow uploads of
larger app bundles) and too aggressive when the workspace is
rate-limiting (we want to dial it down without recompiling).

This change:

- Adds Concurrency to libs/sync.SyncOptions, normalised to
  MaxRequestsInFlight in sync.New when unset so existing callers don't
  need updating.
- Threads the limit through groupRunParallel/applyDiff in
  libs/sync/watchdog.go; the constant is kept as the documented default.
- Surfaces a --concurrency int flag on the two user-facing entry points
  that drive this code path: 'databricks sync' and 'databricks bundle
  sync' (the latter is what backs the apps deploy/sync flow).
- Validates --concurrency >= 1 at flag-parse time per the project rule
  on rejecting incompatible inputs early.
- Adds TestGroupRunParallelHonorsLimit to lock in the parallelism cap.

Slack thread:
https://replit.slack.com/archives/C0A2Z9042FR/p1778082184841149
@luketchang luketchang closed this May 6, 2026
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/cmd/bundle/ - needs approval

Files: cmd/bundle/sync.go
Suggested: @pietern
Also eligible: @denik, @shreyas-goenka, @anton-107, @andrewnester, @janniklasrose, @lennartkats-db

/cmd/sync/ - needs approval

Files: cmd/sync/sync.go
Suggested: @simonfaltum
Also eligible: @renaudhartert-db, @hectorcast-db, @parthban-db, @tanmay-db, @Divyansh-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

/libs/sync/ - needs approval

Files: libs/sync/sync.go, libs/sync/watchdog.go, libs/sync/watchdog_test.go
Suggested: @simonfaltum
Also eligible: @renaudhartert-db, @hectorcast-db, @parthban-db, @tanmay-db, @Divyansh-db, @tejaskochar-db, @mihaimitrea-db, @chrisst, @rauchy

Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db) can approve all areas.
See OWNERS for ownership rules.

@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 5196
  • Commit SHA: c50e7521a76d0968e6f1a52b35a5aea2e96ce78b

Checks will be approved automatically on success.

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.

1 participant