Skip to content

Conversation

@FxKu
Copy link
Member

@FxKu FxKu commented Aug 13, 2024

in our internal e2e tests I found the following logs:

level=warning msg="database replication slots for streams with applicationId test-app not in sync, skipping event stream sync" 
level=warning msg="database replication slots for streams with applicationId test-app-2 not in sync, skipping event stream sync"

The stream with test-app-2 uses a slot with not existing schema and table hence the expected second log line. But the stream of the first app should be fine. I have extended and improved the TestHasSlotsInSync unit test but found no bugs. After adding a logger, I found the issue:

In #2704 we removed always setting a slot to sync in order not block syncing broken streams. However, now they are only set on create, update or delete, but not on a normal sync. Therefore, hasSlotsInSync does not find the desired slots in slotsToSync and skips the stream sync. The proposed solution sets a slot if

If we find out later that the publication can be removed it is set to nil. This code already exists, too.

I'm using this PR to also introduce diffs for annotations and owner references in streams. It required some rewriting of unit tests.

The changes on maintenanceWindows function were triggered because the fake stream client was used before which wasn't working anymore after my changes. First, I choose another client from the same go files (newFakeK8sAnnotationsClient) but then I realized, it's better to just pass the maintenance windows from the manifest to this function to simplify the code.

@FxKu FxKu added the bugfix label Aug 13, 2024
@FxKu FxKu added this to the 1.13.0 milestone Aug 13, 2024
@FxKu FxKu changed the title extend and improve hasSlotsInSync unit test fix sync streams and add diffs for annotations and owner references Aug 14, 2024
@idanovinda
Copy link
Member

👍

1 similar comment
@FxKu
Copy link
Member Author

FxKu commented Aug 14, 2024

👍

@FxKu FxKu merged commit c7ee34e into master Aug 14, 2024
@FxKu FxKu deleted the syncstream-fix branch August 14, 2024 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants