Skip to content

Fix gpexpand 'SyncPackages object has no attribute ret': enqueue OperationWorkerPool operations via items= - #1826

Open
talmacschen-arch wants to merge 1 commit into
apache:mainfrom
talmacschen-arch:fix/operationworkerpool-items-kwarg
Open

Fix gpexpand 'SyncPackages object has no attribute ret': enqueue OperationWorkerPool operations via items=#1826
talmacschen-arch wants to merge 1 commit into
apache:mainfrom
talmacschen-arch:fix/operationworkerpool-items-kwarg

Conversation

@talmacschen-arch

Copy link
Copy Markdown
Contributor

Summary

Fixes #1825.

OperationWorkerPool.__init__ passed its operations positionally to WorkerPool.__init__:

super(OperationWorkerPool, self).__init__(numWorkers, operations)

But WorkerPool.__init__'s second positional parameter is should_stop, and the work items are the third parameter items:

def __init__(self, numWorkers=16, should_stop=False, items=None, ...):
    ...
    if items is not None:
        for item in items:
            self.addCommand(item)

So operations bound to should_stop (a truthy list) and items stayed None. Consequently no operation was ever added to the work queue, should_stop was set truthy, the worker only ever received the halt command, and the submitted operations never ran. Operation.run() therefore never assigned self.ret, and callers hit:

AttributeError: 'SyncPackages' object has no attribute 'ret'

This breaks the first stage of gpexpand ("Syncing Apache Cloudberry extensions"): the exception is squashed into a WARNING, and — more importantly — package/extension synchronization to new segment hosts silently never runs.

Fix

Pass the operations as the items keyword argument so they are enqueued and should_stop stays False:

super(OperationWorkerPool, self).__init__(numWorkers, items=operations)

This re-applies commit cd3c88f6e1e ("Fix gppkg error: 'SyncPackages' object has no attribute 'ret'."), which was inadvertently reverted by the Merge tag 'REL_16_9' into Cloudberry merge (0f4cf8d5068). On current main, grep -c "items=operations" gpMgmt/bin/gppylib/commands/base.py returns 0.

Testing

  • Single-host gpexpand -i <input> -v first stage no longer raises the AttributeError; the SyncPackages operation now actually executes (the -v log shows the worker receiving the operation instead of only a halt command).

See #1825 for the full root-cause analysis and the regression history.

OperationWorkerPool.__init__ passed its operations positionally to
WorkerPool.__init__, whose second positional parameter is should_stop
(items is the third). As a result operations were bound to should_stop
and items stayed None, so no operation was ever added to the work queue
and should_stop was set truthy. The worker only received the halt
command, ParallelOperation's operations never ran, Operation.run() never
assigned self.ret, and callers hit:

    AttributeError: 'SyncPackages' object has no attribute 'ret'

This breaks gpexpand's first stage ("Syncing Apache Cloudberry
extensions"): the exception is squashed to a WARNING and, more
importantly, package/extension sync to new segment hosts silently never
runs.

This re-applies the fix from commit cd3c88f, which was reverted by
the REL_16_9 merge (0f4cf8d). Pass operations as the items keyword
so they are enqueued and should_stop stays False.
@leborchuk

Copy link
Copy Markdown
Contributor

We have lost fix https://github.com/apache/cloudberry/pull/1716/changes while rebasing. Maybe we should just cherry-pick fix from PG14_ARCHIVE to main branch?

@tuhaihe

tuhaihe commented Jun 22, 2026

Copy link
Copy Markdown
Member

We have lost fix https://github.com/apache/cloudberry/pull/1716/changes while rebasing. Maybe we should just cherry-pick fix from PG14_ARCHIVE to main branch?

+1.

@xinzweb xinzweb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified this against current main rather than taking the description at face value:

  • WorkerPool.__init__(self, numWorkers=16, should_stop=False, items=None, ...) — confirmed by reading gpMgmt/bin/gppylib/commands/base.py directly; items is the 3rd positional param.
  • OperationWorkerPool.__init__ on main (line 233) currently calls super().__init__(numWorkers, operations) — positional, so operations binds to should_stop and items stays None. This exactly reproduces the reported bug: nothing gets enqueued, should_stop becomes a truthy list, and the worker only ever receives the halt command.
  • The "this was already fixed once and silently reverted" claim checks out: commit cd3c88f6e1e (2026-05-02) made this exact same items=operations change, and it's absent from main after the Merge tag 'REL_16_9' into Cloudberry merge (0f4cf8d5068, 2026-05-28) — confirmed both commits' SHAs, dates, and the merge's parents.
  • The fix (items=operations as a keyword arg) is correct against the actual WorkerPool signature.

Non-blocking note: there's no unit test covering OperationWorkerPool (no test_base.py under gpMgmt/bin/gppylib/test/unit), and this exact bug has now silently regressed once already via a merge with nothing to catch it. Might be worth a small follow-up test asserting the work queue actually gets populated — not blocking this fix, which is minimal and correct as-is.

CI is green across the board (build/RPM/DEB/rat-check/ic-*). LGTM.

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.

gpexpand first stage fails: 'SyncPackages' object has no attribute 'ret' (extension sync never runs; regression)

4 participants