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
🐛 Destination snowflake: use 200MB batches #34502
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
@@ -37,16 +37,6 @@ class AsyncFlush implements DestinationFlushFunction { | |||
private final long optimalBatchSizeBytes; | |||
private final boolean useDestinationsV2Columns; | |||
|
|||
public AsyncFlush(final Map<StreamDescriptor, WriteConfig> streamDescToWriteConfig, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this constructor was only ever called from the overload createAsync() method. Delete it.
c819387
to
45962b6
Compare
.../java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingDestination.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nit (not a blocker) but overall looks good. One step towards cleaning this class 😅
/publish-java-cdk
|
/publish-java-cdk force=true
|
closes #31741
test sync running - https://cloud.airbyte.com/workspaces/b61bc266-ef3c-460c-af2f-f70da4a2993c/connections/a54f466c-83a6-40e7-ae86-6e79961040d5/job-history#7796162::0. Verified that it now has 200MB batches:
Previous syncs took 500 - 800 COPY statements for the 10M faker dataset; new syncs are taking around 70 COPYs. Total sync runtime is still a bit under 1 hour.
make the factory class more useful; add a builder. Move the 50MB default into the factory class.
new StagingConsumerFactory.Builder()
sounds dumb, I'm open to naming suggestions 🤷