Bulk Load CDK: Staging refactor + tests; multi-sync ITs correctly wait for ack #48608
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
octavia-squidington-iii
added
area/connectors
johnny-schmidt
force-pushed
the
jschmidt/s3v2/issue-10732/staging-tests
branch
4 times, most recently
from
010cab1 to
445ce4b
Compare
johnny-schmidt
changed the title
johnny-schmidt
force-pushed
the
jschmidt/s3v2/issue-10732/staging-tests
branch
from
f574cd5 to
8cd20df
Compare
|
fixed the failing mock dest IT + rebased |
There was a problem hiding this comment.
tests failing in timeout - that's probably expected, given that we're pushing huge data per test. Could try bumping the timeout in destination-s3-v2/gradle.properties, there should be examples in some other destination
(or reduce the number of messages, if we're flushing on every message)
| rate-ms: 900000 # 15 minutes | ||
| window-ms: 900000 # 15 minutes | ||
| destination: | ||
| record-batch-size: 1 # 1 byte for testing; 1 record => 1 upload |
There was a problem hiding this comment.
actually, if we're doing this - do we still need the millions of records thing?
There was a problem hiding this comment.
I disabled that?
There was a problem hiding this comment.
ah derp, I didn't read that file b/c I thought it was just my diff. this makes sense!
(... though I'm not sure why we're hitting test timeouts then)
| @@ -79,7 +80,8 @@ class DefaultSyncManager( | |||
| stream: DestinationStream.Descriptor | |||
| ): StreamLoader? { | |||
| val completable = streamLoaders[stream] | |||
| return completable?.let { if (it.isCompleted) it.await() else null } | |||
| // `.isCompleted` does not work as expected here. | |||
There was a problem hiding this comment.
just curious: what was broken about it?
There was a problem hiding this comment.
It did not return true sometimes even when the loader was clearly completed.
|
@edgao The docker tests were timing out because they needed to consume the batch size limitation via an env variable. I hacked it in. Also they don't throw unclean exit on kill, so I hacked in an exception. Very hack all the way, but they should pass now. |
johnny-schmidt
force-pushed
the
jschmidt/s3v2/issue-10732/staging-tests
branch
from
8ead702 to
8c05dc8
Compare
johnny-schmidt
force-pushed
the
jschmidt/s3v2/issue-10732/staging-tests
branch
from
b65a21f to
88205b7
Compare
What
First three commits are modifications to state so that it is answering specific questions rather than being inspected
Also tests around ensuring those questions are answered
Next two move staging from processBatch to close and disable the tests that are incorrectly not awaiting a checkpoint ack
Next 6 are @edgao 's test fixes to make the multi-sync tests wait on ack
Last one tweaks that to run more efficiently and not use filler records; it also ensures that destination state is persisted after each file is written to staging, so that orphaned staged data after a failure is recovered
EDIT
Plus