Skip to content

feat(pyamber): preserve overlapping channel pauses - #8286

Open
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/preserve-overlapping-channel-pauses
Open

feat(pyamber): preserve overlapping channel pauses#8286
carloea2 wants to merge 1 commit into
apache:mainfrom
carloea2:fix/preserve-overlapping-channel-pauses

Conversation

@carloea2

@carloea2 carloea2 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Release a specifically paused input channel only when no other pause type still holds it.

Any related issues, documentation, discussions?

Closes #8284

How was this PR tested?

Added real-queue coverage for two pause types holding the same channel, including the intermediate disabled state and final release.

C:\Users\carlo\texera\texera\.venv312\Scripts\python.exe -c "import sys,pytest; sys.path[:0]=[r'C:\Users\carlo\texera\texera-worktrees\investigate-bug77\amber\src\main\python',r'C:\Users\carlo\texera\texera\amber\src\main\python']; raise SystemExit(pytest.main([r'amber\src\test\python\core\architecture\managers\test_pause_manager.py','-p','no:cacheprovider','-q']))"

C:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe check amber/src/main/python/core/architecture/managers/pause_manager.py amber/src/test/python/core/architecture/managers/test_pause_manager.py

C:\Users\carlo\texera\texera\.venv312\Scripts\ruff.exe format --check amber/src/main/python/core/architecture/managers/pause_manager.py amber/src/test/python/core/architecture/managers/test_pause_manager.py

All 12 tests passed. Ruff checks passed.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Codex

@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 31, 2026
@github-actions
github-actions Bot requested a review from xuang7 August 31, 2026 08:49
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
⚠️ release/v1.3 Not labeled automatically — none of the files this PR modifies exist on this branch (amber/src/main/python/core/architecture/managers/pause_manager.py, amber/src/test/python/core/architecture/managers/test_pause_manager.py). The fix may target code that isn't on this release, or the files were moved/renamed after the branch was cut. Please check and add release/v1.3 by hand if this fix should be backported here.
release/v1.2 Already labeled — this fix is queued to backport here.

Auto-label run.

@github-actions

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang, @aglinxinyuan
    You can notify them by mentioning @Yicong-Huang, @aglinxinyuan in a comment.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.93%. Comparing base (50321e4) to head (dd1cc54).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8286   +/-   ##
=========================================
  Coverage     93.93%   93.93%           
  Complexity     4761     4761           
=========================================
  Files          1191     1191           
  Lines         48308    48308           
  Branches       5377     5377           
=========================================
  Hits          45378    45378           
  Misses         1495     1495           
  Partials       1435     1435           
Flag Coverage Δ *Carryforward flag
access-control-service 81.00% <ø> (ø) Carriedforward from 50321e4
agent-service 99.32% <ø> (ø) Carriedforward from 50321e4
amber 90.02% <ø> (ø) Carriedforward from 50321e4
computing-unit-managing-service 73.67% <ø> (ø) Carriedforward from 50321e4
config-service 86.86% <ø> (ø) Carriedforward from 50321e4
file-service 87.91% <ø> (ø) Carriedforward from 50321e4
frontend 96.41% <ø> (ø) Carriedforward from 50321e4
notebook-migration-service 79.31% <ø> (ø) Carriedforward from 50321e4
pyamber 98.87% <100.00%> (ø)
workflow-compiling-service 77.19% <ø> (ø) Carriedforward from 50321e4

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 4 better · 🔴 3 worse · ⚪ 8 noise (<±5%) · 0 without baseline

Compared against main 50321e4 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 524 0.32 18,571/24,764/24,764 us 🟢 -13.0% / 🔴 +58.9%
🟢 bs=100 sw=10 sl=64 1,110 0.677 88,266/101,931/101,931 us 🟢 -24.3% / 🟢 -12.3%
🔴 bs=1000 sw=10 sl=64 1,220 0.745 815,037/907,615/907,615 us 🔴 +5.5% / 🟢 +17.1%
Baseline details

Latest main 50321e4 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 524 tuples/sec 547 tuples/sec 787.98 tuples/sec -4.2% -33.5%
bs=10 sw=10 sl=64 MB/s 0.32 MB/s 0.334 MB/s 0.481 MB/s -4.2% -33.5%
bs=10 sw=10 sl=64 p50 18,571 us 17,608 us 12,593 us +5.5% +47.5%
bs=10 sw=10 sl=64 p95 24,764 us 28,477 us 15,579 us -13.0% +58.9%
bs=10 sw=10 sl=64 p99 24,764 us 28,477 us 18,786 us -13.0% +31.8%
bs=100 sw=10 sl=64 throughput 1,110 tuples/sec 1,095 tuples/sec 1,008 tuples/sec +1.4% +10.1%
bs=100 sw=10 sl=64 MB/s 0.677 MB/s 0.668 MB/s 0.615 MB/s +1.3% +10.1%
bs=100 sw=10 sl=64 p50 88,266 us 87,299 us 100,701 us +1.1% -12.3%
bs=100 sw=10 sl=64 p95 101,931 us 134,731 us 107,244 us -24.3% -5.0%
bs=100 sw=10 sl=64 p99 101,931 us 134,731 us 116,122 us -24.3% -12.2%
bs=1000 sw=10 sl=64 throughput 1,220 tuples/sec 1,257 tuples/sec 1,042 tuples/sec -2.9% +17.1%
bs=1000 sw=10 sl=64 MB/s 0.745 MB/s 0.767 MB/s 0.636 MB/s -2.9% +17.1%
bs=1000 sw=10 sl=64 p50 815,037 us 793,180 us 981,959 us +2.8% -17.0%
bs=1000 sw=10 sl=64 p95 907,615 us 860,700 us 1,023,080 us +5.5% -11.3%
bs=1000 sw=10 sl=64 p99 907,615 us 860,700 us 1,051,697 us +5.5% -13.7%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,381.55,200,128000,524,0.320,18571.07,24763.60,24763.60
1,100,10,64,20,1801.85,2000,1280000,1110,0.677,88266.34,101931.02,101931.02
2,1000,10,64,20,16387.25,20000,12800000,1220,0.745,815036.67,907615.14,907615.14

renovate-bot pushed a commit to renovate-bot/apache-_-texera that referenced this pull request Aug 31, 2026
### What changes were proposed in this PR?

One bullet in `amber/LICENSE-binary-python`, in the BSD 3-Clause
section:

```
  - click==8.4.2
+ - cloudpickle==3.1.2
  - contourpy==1.3.3
```

**Why it is a bug.** `build / pyamber (ubuntu-latest, 3.12)` has been
red
repo-wide since ~09:00-13:00 UTC on 2026-08-31, on every open PR
regardless of
what the PR touches, because **joblib 1.6.0 stopped vendoring
cloudpickle**:

| | joblib 1.5.3 | joblib 1.6.0 |
| --- | --- | --- |
| `joblib/externals/cloudpickle/` in the wheel | present (vendored) |
**gone** |
| `Requires-Dist` | *(none)* | `cloudpickle>=3.0` |
| visible to `pip-licenses` | no | **yes** |

`amber/operator-requirements.txt` reaches joblib transitively, so pip
now
installs cloudpickle as its own distribution and the manifest no longer
describes what the image bundles:

```
operator-requirements.txt -> scikit-learn==1.7.2 -> joblib>=1.2.0 -> cloudpickle>=3.0
```

```
Before:  any PR -> pyamber 3.12 -> "+ cloudpickle==3.1.2"                -> red
After:   any PR -> pyamber 3.12 -> "OK: 110 Python packages match ..."   -> green
```

Note that the code was always in the distribution -- it shipped inside
the
joblib wheel. What changed is that it became a separate distribution,
and so
became visible to the license scanner.

**License clearance.** cloudpickle 3.1.2's wheel METADATA says
`License: BSD-3-Clause` (PyPI's classifier reports only the generic
`BSD License`, which is what the CI line quotes), and the bundled
`LICENSE` is
the canonical three-clause text. That is **ASF Category A**. The wheel
ships no
`NOTICE`, and `licenses/LICENSE-BSD-3-Clause.txt` already carries the
text that
the section references, so no `NOTICE-binary` / `NOTICE-binary-python`
change is
needed and the one bullet is the entire fix.

**Deliberately out of scope.** The same step prints a long
`DRIFT (transitive, informational)` list (`charset-normalizer`, `click`,
`joblib`, `regex`, `scipy`, ...). PR runs pass
`--ignore-transitive-version`, so
none of it is fatal, and the nightly exact-match check on `main` owns
refreshing
those pins -- bulk-bumping 22 moving targets here would just add churn.
That is
why `joblib` stays at `1.5.3` in the manifest even though `1.6.0` is
what
introduced the coupling.

This unblocks every open PR's `build / pyamber (ubuntu-latest, 3.12)`
job.

### Any related issues, documentation, discussions?

Closes apache#8292

Related: apache#8294

### How was this PR tested?

Reproduced the CI check locally, byte-for-byte, without a full `pip
install`.
CI's own report is a complete description of the diff between the
manifest and
reality (1 `+`, 0 stale, 0 direct drift, 22 transitive drifts), so the
installed
set was rebuilt as "every claimed bullet at its bundled version, plus
cloudpickle==3.1.2" and fed to the checker exactly as `build.yml`
invokes it:

```bash
python bin/licensing/check_binary_deps.py --ignore-transitive-version python /tmp/pip-licenses.csv
```

| | exit | output |
| --- | --- | --- |
| before (main) | 1 | `+ cloudpickle==3.1.2 (license: BSD License)` --
identical to run 33394216939 |
| after (this PR) | 0 | `OK: 110 Python packages match LICENSE-binary.`
|

The `--ignore-transitive-version`-less (nightly) mode still exits 1 on
the 22
transitive drifts both before and after this change, so the behavior
there is
unchanged -- that is the nightly refresh's job, not this PR's.

Upstream cause confirmed straight from the published wheels:

```bash
pip download joblib==1.5.3 --no-deps -d . && pip download joblib==1.6.0 --no-deps -d .
# 1.6.0 METADATA: Requires-Dist: cloudpickle>=3.0, and no joblib/externals/cloudpickle/
# 1.5.3 METADATA: no Requires-Dist,           and joblib/externals/cloudpickle/ present
pip download cloudpickle==3.1.2 --no-deps -d .
# METADATA: License: BSD-3-Clause; licenses/LICENSE is the 3-clause text; no NOTICE
```

The failure was verified as repo-wide rather than PR-specific on two
unrelated
branches -- `chore/remove-superseded-sql-updates` (run 33394216939,
12:55 UTC)
and `ci/8084-backport-manager-approval-gate` (run 33392828233, 12:39
UTC) -- both
failing on the same `+ cloudpickle==3.1.2` line, against green license
steps at
08:39 UTC (apache#8282) and 08:49 UTC (apache#8286).

One wrinkle worth flagging for the reviewer: `build` was **skipped** on
this
PR's first runs. `amber/LICENSE-binary-python` matches no glob in
`.github/labeler.yml`, so no stack label was applied, `precheck`
selected no
stacks, and the aggregate gate accepts `build: skipped` as passing --
i.e. a
change to this manifest is not normally gated by the check that
validates it.
The `pyamber` label was applied by hand here so the license check
actually runs
against the fix. That gap is filed separately as apache#8294 and left out of
this
diff to keep the hotfix a one-liner.

### Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)
@carloea2 carloea2 changed the title fix(pyamber): preserve overlapping channel pauses feat(pyamber): preserve overlapping channel pauses Aug 31, 2026
@mengw15 mengw15 removed the release/v1.2 back porting to release/v1.2 label Aug 31, 2026
@xuang7
xuang7 removed their request for review September 1, 2026 01:37
@xuang7 xuang7 removed the fix label Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Releasing one channel pause ignores another active hold

5 participants