Skip to content
This repository has been archived by the owner on Sep 23, 2024. It is now read-only.

Consider flush_all_streams for schema updates #247

Conversation

mvgijssel
Copy link
Contributor

@mvgijssel mvgijssel commented Feb 3, 2022

Problem

Due to a high number of schema messages due to handling TOAST messages in transferwise/pipelinewise-tap-postgres#146 all the records are constantly flushed resulting in very slow performance.

Proposed changes

This changes ensures the flush_all_streams configuration setting is honored when a new schema message arrives in the target. If this is not set, then only the records for the current stream are flushed instead for all rows.

Types of changes

What types of changes does your code introduce to PipelineWise?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Checklist

  • Description above provides context of the change
  • I have added tests that prove my fix is effective or that my feature works
  • Unit tests for changes (not needed for documentation changes)
  • CI checks pass with my changes
  • Bumping version in setup.py is an individual PR and not mixed with feature or bugfix PRs
  • Commit message/PR title starts with [AP-NNNN] (if applicable. AP-NNNN = JIRA ID)
  • Branch name starts with AP-NNN (if applicable. AP-NNN = JIRA ID)
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions

Copy link
Contributor

@Samira-El Samira-El left a comment

Choose a reason for hiding this comment

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

Thanks @mvgijssel, this makes sense! not sure why it was like this in the first place 😅 .

@Samira-El
Copy link
Contributor

Can you rebase the branch so I can merge?

@mvgijssel
Copy link
Contributor Author

Can you rebase the branch so I can merge?

Will do!

@mvgijssel
Copy link
Contributor Author

GitHub shows me

First-time contributors need a maintainer to approve running workflows.

So once you approve the workflow run it should be good :)

@Samira-El Samira-El merged commit bde2b05 into transferwise:master Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants