Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always enter parallel mode when scanning a table in parallel #604

Merged
merged 1 commit into from
Feb 14, 2025
Merged

Conversation

JelteF
Copy link
Collaborator

@JelteF JelteF commented Feb 14, 2025

Previously we were only calling EnterParallelMode() if we were not already in parallel mode. And then when we were done with that the scan for which we entered parallel mode, we would call ExitParallelMode(). This had the problem that if we were doing two scans, we could call ExitParallelMode() before both scans were done:

  1. Scan A starts: EnterParallelMode
  2. Scan B starts: do nothing
  3. Scan A exists: ExitParallelMode
  4. Scan B is now running in parallel even though we exited parallel mode.

This was causing an assertion in the Postgres code to fail.

This change fixes that by always calling EnterParallelMode() when we do a parallel scan and always calling ExitParallelMode() when we are finished with a scan. This works because Postgres internally keeps a counter of the amount of times we entered parallel mode. So now in the above example we would do:

  1. Scan A starts: EnterParallelMode(), counter 1, enter parallel mode
  2. Scan B starts: EnterParallelMode(), counter 2, do nothing
  3. Scan A exists: ExitParallelMode(), counter 1, do nothing
  4. Scan B exists: ExitParallelMode(), counter 0, exit parallel mode

Fixes #592

Previously we were only calling `EnterParallelMode()` if we were not
already in parallel mode. And then when we were done with that the scan
for which we entered parallel mode, we would call `ExitParallelMode()`.
This had the problem that if we were doing two scans, we could call
ExitParallelMode() before both scans were done:

1. Scan A starts: EnterParallelMode
2. Scan B starts: do nothing
3. Scan A exists: ExitParallelMode
4. Scan B is now running in parallel even though we exited parallel
   mode.

This was causing an assertion in the Postgres code to fail.

This change fixes that by always calling `EnterParallelMode()` when we
do a parallel scan and always calling `ExitParallelMode()` when we are
finished with a scan. This works because Postgres internally keeps a
counter of the amount of times we entered parallel mode. So now in the
above example we would do:

1. Scan A starts: EnterParallelMode(), counter 1, enter parallel mode
1. Scan B starts: EnterParallelMode(), counter 2, do nothing
3. Scan A exists: ExitParallelMode(), counter 1, do nothing
3. Scan B exists: ExitParallelMode(), counter 0, exit parallel mode
@JelteF JelteF requested a review from mkaruza February 14, 2025 14:48
Copy link
Collaborator

@mkaruza mkaruza left a comment

Choose a reason for hiding this comment

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

LGTM

@JelteF JelteF merged commit a344d04 into main Feb 14, 2025
5 checks passed
@JelteF JelteF deleted the fix-592 branch February 14, 2025 15:50
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.

Assertion failure in PostgresTableReaderCleanup about parallel context not being active
2 participants