mlevkov commented on PR #3795: URL: https://github.com/apache/iggy/pull/3795#issuecomment-5311004680
Follow-ups from your review are filed so they survive this PR merging: - #3898 - `producer.send()` failure lets a later batch supersede the skipped save. Same supersession shape, not closed by the latch here since that is armed only by a drop. - #3899 - elasticsearch_source persists its own cursor and overrides the runtime state at open, so a runtime-side guarantee cannot cover it. - #3900 - `spawn_source_handler` at 11 params, blocked on the resolved-path/version split on `SourceConnectorPlugin`. One I took here instead of filing: the discarded `iggy_source_handle` return code (`f2db3b48d`, now `5de9283fa` after the rebase). It is the twin of the close-code check already in this PR, so fixing one and deferring the other seemed worse than the small extra diff. Non-zero means the SDK could not register the handler, which leaves an instance reporting Running while producing nothing. **On the regression test** - the saturate, stop, restart, assert-no-row-gap one against `state_persists_across_connector_restart`: I read "once the latch lands" as meaning a follow-up, so I have not written it. But it is the direct regression test for the fix in this PR, so I would rather ask than guess. Do you want it here, or as a separate PR once this merges? If here, it goes in the postgres integration suite rather than the runtime unit tests, so it will need Docker in the loop. Also rebased onto current master per your note, along with my other three. Gate green on all four; this branch is 134 passing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
