Fix worker launch: add lib-common to the supervisor's worker classpath and restore the lib-worker drop-in directory

[reiabreu]

Background

Storm's distribution is meant to have three classpath directories:

lib/          daemon-exclusive jars  (nimbus, supervisor, UI)
lib-common/   shared jars            (both daemons and workers)
lib-worker/   worker-exclusive jars  (topology worker processes)

Since #8819 the build collapsed this into two directories by moving all worker jars into lib-common, making lib-worker disappear. This introduced a bug: BasicContainer.frameworkClasspath() was never updated and kept pointing at the now-absent lib-worker/, causing workers to fail at launch with ClassNotFoundException: org.apache.storm.LogWriter.

Changes

Bug fix — BasicContainer.java
Worker classpath now correctly references both lib-common/ (shared) and lib-worker/ (worker-exclusive).

Build — pom.xml (final-package)
stage-worker-lib now stages worker jars into staging/lib-worker instead of staging/lib-common, so dedup-libs.py can compute the true intersection rather than a blind merge.

Build — dedup-libs.py + test_dedup_libs.py
Phase 2 now promotes only jars present in both lib and lib-worker to lib-common. Worker-exclusive jars stay in lib-worker. Version drift between lib and lib-worker is intentional and no longer an error (they are separate classpaths). All 14 unit tests updated and passing.

Cleanup — bin/storm.py
Restores STORM_WORKER_LIB_DIR and the client=True branch in get_classpath() to match the 3-way layout. Updates the comment to accurately describe the distribution structure.

[rzo1]

Overall: The BasicContainer fix is correct and needed — thanks for catching it. Workers launched by the supervisor really do get a classpath pointing at the now-absent lib-worker/, and lib-common/* must be added. However, I'd like to see that 2-line fix land on its own, because the build-system changes in this PR re-introduce the 3-way split in a way that isn't fully wired up and trades a build-time safety check for silent jar loss:

1. common.xml — worker-exclusive jars never reach the distribution (blocking).
The pom now stages worker jars into staging/lib-worker and dedup-libs.py intentionally leaves worker-exclusive jars there, but the assembly descriptor (src/main/assembly/common.xml) only has fileSets for staging/lib and staging/lib-common. There is no lib-worker fileSet, so any jar that stays in lib-worker is silently dropped from the tarball. The 3-way layout this PR describes never actually materializes in the final distribution — it only appears to work because the worker classpath is currently a strict subset of the daemon classpath, leaving lib-worker empty. If we keep the 3-way approach, common.xml needs a staging/lib-worker → lib-worker fileSet (and the stale comment there, "worker jars now live in lib-common", needs updating).

2. dedup-libs.py — dropping the version-drift failure is a regression (blocking, combined with #1).
On master, version drift between the daemon and worker jar sets fails the build deliberately: in a single-reactor build the worker set is a subset of the daemon set by construction, so drift signals a build misconfiguration. This PR reclassifies drift as "intentional and allowed" — and because of point 1, a drifted worker jar is now silently discarded instead of failing the build. That's strictly worse than the current behavior. I'd keep drift as a hard failure until there's an actual, intended case of a worker-only dependency.

3. PR description vs. actual storm.py diff (minor).
The description says this "restores STORM_WORKER_LIB_DIR and the client=True branch in get_classpath()", but both already exist on master — the actual diff is comment-only. Worth correcting so the change log stays accurate.

Suggestion: split out the BasicContainer.java hunk (plus a supervisor-side test, if feasible — e.g. asserting frameworkClasspath() contains lib-common) as a minimal bug-fix PR so we can merge it quickly, and discuss the 2-way vs. 3-way layout separately. As it stands the extra machinery is dead code in a single-reactor build, and half-wired dead code at that.

[rzo1]

I can do the extraction as well and a small test for it, if you would prefer that way.

[rzo1]

I'll have a look now.

[reiabreu]

Thanks for the quick reply. I'll cancel the vote, no problem.
I agree that just fixing BasicContainer is a quick win.
Regarding version drift, we can discuss it in a separate thread, but as long as daemons and workers have separate classpaths, no conflict should arise.
I'll drop the other changes and just fix BasicContainer

[rzo1]

I am currently doing some tests with just the BasicContainer fix and will report back once done ;-)

[reiabreu]

Feel free to directly edit this branch. Or I can close it and you can open a new one. Thank you

…

On Sun, Jul 19, 2026, 10:36 Richard Zowalla ***@***.***> wrote: *rzo1* left a comment (apache/storm#8905) <#8905 (comment)> I am currently doing some tests with just the BasicContainer fix and will report back once done ;-) — Reply to this email directly, view it on GitHub <#8905?email_source=notifications&email_token=AAG5GIVRWKRODDPIMTWEEPL5FSJCHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBRGUZDENRUGAZ2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5015226403>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG5GIQBOKPP62U4PF7OIUT5FSJCHAVCNFSNUABEKJSXA33TNF2G64TZHMYTIMJTGU2DOMB3JFZXG5LFHM2DSMRRHEZDQMZZGCQXMAQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[rzo1]

Thanks @reiabreu! To save you some work and keep this moving quickly, here's what I plan to do on this branch:

1. Keep your BasicContainer fix as the core of this PR (once you've dropped the build-system changes, or I can do that while I'm at it).

2. Push a regression test on top: the existing launch tests mock frameworkClasspath() away (FRAMEWORK_CP), which is exactly why this regression went unnoticed — the new test exercises the real implementation and asserts lib-common/* and lib-worker/* are on the worker classpath.

3. Push a small build change restoring lib-worker/ as an empty drop-in directory (README only) in both distributions. Operators use it to inject worker-only jars, and both storm.py and BasicContainer keep it on the worker classpath — this preserves that extension point and part of the familiar 2.x layout without re-introducing the 3-way dedup machinery.

[reiabreu]

1 - feel free to drop them Thank you

…

On Sun, 19 Jul 2026 at 10:52, Richard Zowalla ***@***.***> wrote: *rzo1* left a comment (apache/storm#8905) <#8905 (comment)> Thanks @reiabreu <https://github.com/reiabreu>! To save you some work and keep this moving quickly, here's what I plan to do on this branch: 1. Keep your BasicContainer fix as the core of this PR (once you've dropped the build-system changes, or I can do that while I'm at it). 2. Push a regression test on top: the existing launch tests mock frameworkClasspath() away (FRAMEWORK_CP), which is exactly why this regression went unnoticed — the new test exercises the real implementation and asserts lib-common/* and lib-worker/* are on the worker classpath. 3. Push a small build change restoring lib-worker/ as an empty drop-in directory (README only) in both distributions. Operators use it to inject worker-only jars, and both storm.py and BasicContainer keep it on the worker classpath — this preserves that extension point and part of the familiar 2.x layout without re-introducing the 3-way dedup machinery. — Reply to this email directly, view it on GitHub <#8905?email_source=notifications&email_token=AAG5GITDQSMYX3PFZWACMXT5FSK6XA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMBRGUZDOMBTGQ22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5015270345>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG5GISCE6XRHPVPVJMZMG35FSK6XAVCNFSNUABEKJSXA33TNF2G64TZHMYTIMJTGU2DOMB3JFZXG5LFHM2DSMRRHEZDQMZZGCQXMAQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[rzo1]

So all good. Run the explamation topology on a docker based cluster setup build from this branch. Works fine.

[reiabreu]

Thanks for the quick turnaround! I'll have a RC2 out in a few hours

…

On Sun, 19 Jul 2026 at 11:25, Richard Zowalla ***@***.***> wrote: ***@***.**** approved this pull request. So all good. Run the explamation topology on a docker based cluster setup build from this branch. Works fine. — Reply to this email directly, view it on GitHub <#8905?email_source=notifications&email_token=AAG5GIS2CLHNGKWGL7Z75C35FSOZVA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZTGA2TQMRRHA42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4730582189>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG5GIVTJ545UAOKLKABYLL5FSOZVAVCNFSNUABEKJSXA33TNF2G64TZHMYTIMJTGU2DOMB3JFZXG5LFHM2DSMRRHEZDQMZZGCQXMAQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[rzo1]

Thanks for the quick turnaround! I'll have a RC2 out in a few hours
…
On Sun, 19 Jul 2026 at 11:25, Richard Zowalla @.> wrote: @.* approved this pull request. So all good. Run the explamation topology on a docker based cluster setup build from this branch. Works fine. — Reply to this email directly, view it on GitHub <#8905?email_source=notifications&email_token=AAG5GIS2CLHNGKWGL7Z75C35FSOZVA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINZTGA2TQMRRHA42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4730582189>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG5GIVTJ545UAOKLKABYLL5FSOZVAVCNFSNUABEKJSXA33TNF2G64TZHMYTIMJTGU2DOMB3JFZXG5LFHM2DSMRRHEZDQMZZGCQXMAQ . You are receiving this because you were mentioned.Message ID: @.***>

Thanks for spotting + release prep!