S3 Destination: Descreased thread allocation & memory ratio for AsyncConsumer

[johnny-schmidt]

This issue

Some configurations OOM consistently in the cloud. I can't find evidence this is affecting customers, but bumping memory to 2G (same as other async destinations) seems to enable it to succeed.

There are three changes here:

• increase memory requirements to 2G
• reduce the memory ratio to 0.5
• additional changes to make it possible to tweak memory ratio and concurrency entirely with a connector release (no CDK)

I built a dev connector off this, and it allows even the most demanding loads to make steady progress.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Aug 13, 2024 0:34am

[johnny-schmidt]

Successful sync with this config: 5e7102b0b1e7e19b14650c8b0c6b976f95687079!

[evantahler]

Ideally, the way that connectors work is that they work in lower-memory environments, but they go slower (e.g. smaller batches or less parallelism). This is good both for our cloud costs, and for self-hosted users that don't have large K8s clusters.

As most syncs seem to work with the 1gb setting, I'd suggest that we delay rolling this out for all cloud syncs unless there really is no other option. E.g. Maybe the parallelism for the S3 destination needs to be less, or the % of memory we allocate to an async worker should be increased?

[edit] somehow I was commenting on some phantom version of this PR that didn't also include the thread / memory % changes. Are just those enough, or do we also need the 2gb bump for sync to succeed?

[evantahler]

Should this be some function of the number of CPUs that are allocated to the container?

[johnny-schmidt]

Probably, but it's hardcoded to five currently. Also according to the resource runbook, the syncs only have 1 core by default? I assumed the point was to parallelize workloads that we assumed would be network bound (but because we're staging everything to a local file before syncing, that's probably less true?)

[johnny-schmidt]

@evantahler I reverted the memory increase request. Now this change

• reduces the memory ratio from 0.7 to 0.5
• reduces the number of worker threads from 5 to 1

I will confirm this works for all configurations against the benchmark w/o significant perf reduction.

[gisripa]

No blocking comments,
Not sure if the having a default of Runtime.availableProcessors or something is sane or even that is not good for Parquet.

[johnny-schmidt]

No blocking comments, Not sure if the having a default of Runtime.availableProcessors or something is sane or even that is not good for Parquet.

@gisripa judging by the orchestrator logs, that's going to be 1 anyway