Skip to content

FLOW-7395 React to channel error count #1232

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.

Sign up for GitHub

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

Jump to bottom
Merged
sfc-gh-lkucharski merged 2 commits into from
Dec 4, 2025
Merged

FLOW-7395 React to channel error count #1232

sfc-gh-lkucharski merged 2 commits into from
Dec 4, 2025

Conversation

@sfc-gh-lkucharski
Copy link
Contributor

@sfc-gh-lkucharski sfc-gh-lkucharski commented Dec 2, 2025 •
edited
Loading

Overview

FLOW-7395 React to channel error count

Pre-review checklist

  • This change should be part of a Behavior Change Release. See go/behavior-change.
  • This change has passed Merge gate tests
  • Snowpipe Changes
  • Snowpipe Streaming Changes
  • This change is TEST-ONLY
  • This change is README/Javadocs only
  • This change is protected by a config parameter <PARAMETER_NAME> eg snowflake.ingestion.method.
    • Yes - Added end to end and Unit Tests.
    • No - Suggest why it is not param protected
  • Is his change protected by parameter <PARAMETER_NAME> on the server side?
    • The parameter/feature is not yet active in production (partial rollout or PrPr, see Changes for Unreleased Features and Fixes).
    • If there is an issue, it can be safely mitigated by turning the parameter off. This is also verified by a test (See go/ppp).

@sfc-gh-lkucharski sfc-gh-lkucharski mentioned this pull request Dec 2, 2025
Merged
12 tasks
@sfc-gh-lkucharski Graphite App
Copy link
Contributor Author

sfc-gh-lkucharski commented Dec 2, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sfc-gh-lkucharski sfc-gh-lkucharski mentioned this pull request Dec 2, 2025
Merged
12 tasks
@sfc-gh-lkucharski sfc-gh-lkucharski changed the title Fix the tests FLOW-7395 React to channel error count Dec 2, 2025
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from 64883d8 to 60b9d91 Compare December 2, 2025 21:20
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 3526c08 to dfa462d Compare December 2, 2025 21:20
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from 60b9d91 to e2d9666 Compare December 2, 2025 21:36
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from dfa462d to 614b2cf Compare December 2, 2025 21:36
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from e2d9666 to 3d66650 Compare December 2, 2025 22:51
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 614b2cf to 9a1578a Compare December 2, 2025 22:51
@sfc-gh-lkucharski sfc-gh-lkucharski mentioned this pull request Dec 2, 2025
Merged
12 tasks
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 9a1578a to d7ad3f9 Compare December 2, 2025 23:11
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch 2 times, most recently from 9313fa9 to ef65dc7 Compare December 2, 2025 23:21
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch 2 times, most recently from 6ee0b38 to a53b634 Compare December 3, 2025 01:35
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from ef65dc7 to be5834f Compare December 3, 2025 01:35
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from a53b634 to 7e1b773 Compare December 3, 2025 01:40
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from be5834f to 55ce579 Compare December 3, 2025 02:09
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 7e1b773 to 1b44bbd Compare December 3, 2025 02:09
@sfc-gh-lkucharski sfc-gh-lkucharski marked this pull request as ready for review December 3, 2025 02:32
@sfc-gh-lkucharski sfc-gh-lkucharski requested a review from a team as a code owner December 3, 2025 02:32
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from 55ce579 to 5a911f8 Compare December 3, 2025 13:00
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 1b44bbd to d99a1fc Compare December 3, 2025 13:00
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from 5a911f8 to 01bc9c4 Compare December 3, 2025 13:00
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from d99a1fc to 6e3b256 Compare December 3, 2025 13:01
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from 01bc9c4 to 128048b Compare December 3, 2025 17:51
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 6e3b256 to 3a25fc6 Compare December 3, 2025 17:51
sfc-gh-mbobowski
sfc-gh-mbobowski reviewed Dec 4, 2025
View reviewed changes
src/main/java/com/snowflake/kafka/connector/SnowflakeSinkTask.java
// Check for channel errors detected in preCommit and fail the task
if (this.channelErrorToFailOn != null) {
SnowflakeKafkaConnectorException error = this.channelErrorToFailOn;
this.channelErrorToFailOn = null; // Clear so we don't throw again on restart
Copy link
Contributor

@sfc-gh-mbobowski sfc-gh-mbobowski Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is redundant. After exception is thrown in the next line a task will essentially die and be removed from Kafka Connect state. After restart a new instance is created that will have null set on construction.

sfc-gh-mbobowski
sfc-gh-mbobowski reviewed Dec 4, 2025
View reviewed changes
src/test/java/com/snowflake/kafka/connector/internal/streaming/ChannelStatusCheckIT.java Outdated
channel.setChannelStatus(statusWithErrors);
}

// Produce more messages to trigger preCommit which checks channel status
Copy link
Contributor

@sfc-gh-mbobowski sfc-gh-mbobowski Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to insert any records to trigger precommit. It's time-based operation.

Copy link
Contributor Author

@sfc-gh-lkucharski sfc-gh-lkucharski Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

sfc-gh-mbobowski
sfc-gh-mbobowski reviewed Dec 4, 2025
View reviewed changes
src/test/java/com/snowflake/kafka/connector/internal/streaming/ChannelStatusCheckIT.java Outdated
Map<String, String> workerConfig = new HashMap<>();
workerConfig.put("plugin.discovery", "hybrid_warn");
// Set a short offset flush interval for faster preCommit calls
workerConfig.put("offset.flush.interval.ms", "5000");
Copy link
Contributor

@sfc-gh-mbobowski sfc-gh-mbobowski Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reducing it from 5000 to 1000 speeds up test execution by 8 seconds. Imo it's worth it.

Copy link
Contributor Author

@sfc-gh-lkucharski sfc-gh-lkucharski Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

sfc-gh-mbobowski
sfc-gh-mbobowski approved these changes Dec 4, 2025
View reviewed changes
Copy link
Contributor

@sfc-gh-mbobowski sfc-gh-mbobowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left two comments about redundant code, but generally it's ok.

@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch 2 times, most recently from 3a7e83f to fcb18d2 Compare December 4, 2025 12:12
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 3a25fc6 to 5cb9de0 Compare December 4, 2025 12:12
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 branch from fcb18d2 to f64ffed Compare December 4, 2025 12:14
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 5cb9de0 to b328611 Compare December 4, 2025 12:14
@sfc-gh-lkucharski sfc-gh-lkucharski changed the base branch from lkucharski/FLOW-7155-adapt-iceber-tests-for-ssv2 to graphite-base/1232 December 4, 2025 12:33
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 20996cd to 9ed4eca Compare December 4, 2025 12:33
@graphite-app graphite-app bot changed the base branch from graphite-base/1232 to master December 4, 2025 12:34
@sfc-gh-lkucharski sfc-gh-lkucharski force-pushed the lkucharski/FLOW-7395-react-to-channel-error-count branch from 9ed4eca to b9aecc6 Compare December 4, 2025 12:34
@sfc-gh-lkucharski sfc-gh-lkucharski merged commit c3db8e1 into master Dec 4, 2025
31 checks passed
@sfc-gh-lkucharski sfc-gh-lkucharski deleted the lkucharski/FLOW-7395-react-to-channel-error-count branch December 4, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-mbobowski sfc-gh-mbobowski sfc-gh-mbobowski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sfc-gh-lkucharski @sfc-gh-mbobowski