Skip to content
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 S3V2: Restore Thread-Safety to Unique Key Counter #50989

Merged

Conversation

johnny-schmidt
Copy link
Contributor

What

The changes here removed the access lock around the unique key counter as well as the one around the object list.

This likely caused the unique keys not to be visible across threads, resulting in this failure https://cloud.airbyte.com/workspaces/53a30509-28a4-4158-9c84-200cbad69a25/connections/1e5881b2-1745-437d-8420-d9957a3e4467/timeline?openLogs=true&eventId=403efe8f-a47b-4fb0-81ef-5bdd234394c3. (That exception indicates that multiple process records workers tried to write to the same filename.)

The bug itself caused no harm due to the extra uniqueness check, but it's a simple fix.

(I also cleaned up the alarming log message in the Checker that made me think this was worse than it was. : )

@johnny-schmidt johnny-schmidt requested a review from a team as a code owner January 8, 2025 21:51
Copy link

vercel bot commented Jan 8, 2025

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

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 9, 2025 5:33pm

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues CDK Connector Development Kit connectors/destination/s3 labels Jan 8, 2025
@johnny-schmidt johnny-schmidt force-pushed the jschmidt/s3v2/restore-thread-safety-for-key-counts branch from 83a93e7 to af7d487 Compare January 8, 2025 21:55
@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Jan 8, 2025
@johnny-schmidt johnny-schmidt enabled auto-merge (squash) January 8, 2025 21:56
@johnny-schmidt johnny-schmidt force-pushed the jschmidt/s3v2/restore-thread-safety-for-key-counts branch from bb92632 to 7fd6556 Compare January 8, 2025 23:34
@johnny-schmidt johnny-schmidt force-pushed the jschmidt/s3v2/restore-thread-safety-for-key-counts branch 2 times, most recently from dcfe63f to 814f7cb Compare January 9, 2025 00:45
@johnny-schmidt johnny-schmidt force-pushed the jschmidt/s3v2/restore-thread-safety-for-key-counts branch from 814f7cb to a235199 Compare January 9, 2025 00:57
@johnny-schmidt johnny-schmidt force-pushed the jschmidt/s3v2/restore-thread-safety-for-key-counts branch from a235199 to 85a29df Compare January 9, 2025 16:39
@johnny-schmidt johnny-schmidt enabled auto-merge (squash) January 9, 2025 16:41
@johnny-schmidt johnny-schmidt force-pushed the jschmidt/s3v2/restore-thread-safety-for-key-counts branch from 85a29df to 32fb615 Compare January 9, 2025 17:27
@johnny-schmidt johnny-schmidt merged commit e8bf09d into master Jan 9, 2025
30 checks passed
@johnny-schmidt johnny-schmidt deleted the jschmidt/s3v2/restore-thread-safety-for-key-counts branch January 9, 2025 17:51
@johnny-schmidt johnny-schmidt restored the jschmidt/s3v2/restore-thread-safety-for-key-counts branch January 9, 2025 17:52
@johnny-schmidt
Copy link
Contributor Author

Actually looks like rc.5 got in ahead of you, so you'll need to bump this to rc.6. I'll take care of rolling forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation CDK Connector Development Kit connectors/destination/s3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants