Skip to content

fix: Skip incremental tables when performing delete-stale#13482

Merged
kodiakhq[bot] merged 2 commits intomainfrom
ignore-incremental-tables-for-delete-stale
Aug 30, 2023
Merged

fix: Skip incremental tables when performing delete-stale#13482
kodiakhq[bot] merged 2 commits intomainfrom
ignore-incremental-tables-for-delete-stale

Conversation

@hermanschaaf
Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf commented Aug 30, 2023

This fixes a bug introduced with the migration to SDK v4 / Protocol v3; when overwrite-delete-stale write mode is used, stale entries should not be removed from incremental tables when a state backend is active.

@hermanschaaf hermanschaaf changed the title fix: Ignore incremental tables when performing delete-stale fix: Skip incremental tables when performing delete-stale Aug 30, 2023
Copy link
Copy Markdown
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Looks good, we might need it in sync_v2 too

_, err := destinationsPbClients[i].DeleteStale(ctx, &destination.DeleteStale_Request{

@hermanschaaf hermanschaaf added automerge Automatically merge once required checks pass and removed automerge Automatically merge once required checks pass labels Aug 30, 2023
@hermanschaaf
Copy link
Copy Markdown
Contributor Author

Looks good, we might need it in sync_v2 too

@erezrokah You're probably right; v2 was still a batch protocol so it's quite different, it's not immediately clear to me how we can fix it there. I think I'm going to defer fixing this, and if we get a report of a community v2 source that also uses incremental tables we can take another look (or upgrade the source to v3, which might be easier)

@hermanschaaf hermanschaaf added the automerge Automatically merge once required checks pass label Aug 30, 2023
@kodiakhq kodiakhq bot merged commit 2ce7387 into main Aug 30, 2023
@kodiakhq kodiakhq bot deleted the ignore-incremental-tables-for-delete-stale branch August 30, 2023 16:11
kodiakhq bot pushed a commit that referenced this pull request Aug 30, 2023
🤖 I have created a release *beep* *boop*
---


## [3.16.1](cli-v3.16.0...cli-v3.16.1) (2023-08-30)


### Bug Fixes

* Skip incremental tables when performing delete-stale ([#13482](#13482)) ([2ce7387](2ce7387))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants