Skip to content

Fixed parallel ingestion.#3135

Merged
khustup2 merged 1 commit intomainfrom
fix-race
Feb 10, 2026
Merged

Fixed parallel ingestion.#3135
khustup2 merged 1 commit intomainfrom
fix-race

Conversation

@khustup2
Copy link
Copy Markdown
Contributor

🚀 🚀 Pull Request

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Things to be aware of

Things to worry about

Additional Context

@claude
Copy link
Copy Markdown

claude Bot commented Feb 10, 2026

Claude finished @khustup2's task in 47s —— View job


Review Summary

Found 2 critical memory context management bugs in the subtransaction cleanup code:

  • sync_worker.cpp:141 - Missing MemoryContextSwitchTo before releasing subtransaction
  • table_storage.cpp:396 - Same issue in the success path

Both need the memory context restored before ReleaseCurrentSubTransaction() to prevent memory corruption.

@sonarqubecloud
Copy link
Copy Markdown

elog(LOG, "pg_deeplake sync: successfully created table %s", qualified_name.c_str());
}

ReleaseCurrentSubTransaction();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing MemoryContextSwitchTo(saved_context) before ReleaseCurrentSubTransaction(). PostgreSQL subtransaction cleanup requires restoring the memory context before releasing, or you risk context corruption.

Suggested change
ReleaseCurrentSubTransaction();
MemoryContextSwitchTo(saved_context);
ReleaseCurrentSubTransaction();

PopActiveSnapshot();
}

ReleaseCurrentSubTransaction();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing MemoryContextSwitchTo(saved_context) before ReleaseCurrentSubTransaction(). This can cause memory context corruption when the subtransaction commits successfully.

Suggested change
ReleaseCurrentSubTransaction();
MemoryContextSwitchTo(saved_context);
ReleaseCurrentSubTransaction();

@khustup2 khustup2 merged commit 43fa9d8 into main Feb 10, 2026
6 checks passed
@khustup2 khustup2 deleted the fix-race branch February 10, 2026 03:38
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