Skip to content

IGNITE-28345 Skip client compute notification on executionFut fail#7906

Merged
ptupitsyn merged 3 commits into
apache:mainfrom
ptupitsyn:ignite-28345
Mar 31, 2026
Merged

IGNITE-28345 Skip client compute notification on executionFut fail#7906
ptupitsyn merged 3 commits into
apache:mainfrom
ptupitsyn:ignite-28345

Conversation

@ptupitsyn
Copy link
Copy Markdown
Contributor

@ptupitsyn ptupitsyn commented Mar 31, 2026

  • Client request handler: prevent unexpected notifications when job/task execution fails early
  • CI config: add build failure detection for "Unexpected xxx ID" in .NET to catch those problems early

https://issues.apache.org/jira/browse/IGNITE-28345

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes IGNITE-28345 by preventing client compute result notifications from being sent when the initial job submission (executionFut) fails, avoiding protocol-level “unexpected notification” scenarios on the client side.

Changes:

  • Update compute execute notification flow to skip sending notifications when executionFut completes exceptionally.
  • Add TeamCity Linux .NET log fail conditions to immediately fail builds on “Unexpected notification ID” / “Unexpected response ID”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/compute/ClientComputeExecuteRequest.java Avoids sending compute result notifications when submission fails (no JobExecution available).
.teamcity/test/platform_tests/PlatformDotnetTestsLinux.kt Makes CI fail fast on protocol desync log signatures in .NET platform tests.
Comments suppressed due to low confidence (1)

modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/compute/ClientComputeExecuteRequest.java:144

  • sendResultAndState ignores errState from execution.stateAsync(): if state retrieval fails while the result succeeds, the notification is still sent as successful and the state is packed as nil, which can hide the underlying failure. Consider passing a combined error (first non-null of err and errState) for consistency with ClientComputeExecuteMapReduceRequest (which uses IgniteUtils.firstNotNull).
                execution.resultAsync().whenComplete((val, err) ->
                        execution.stateAsync().whenComplete((state, errState) -> {
                            try {
                                notificationSender.sendNotification(
                                        w -> {
                                            Marshaller<Object, byte[]> marshaller = extractMarshaller(execution);
                                            ClientComputeJobPacker.packJobResult(val, marshaller, w);
                                            packJobState(w, state);
                                        },
                                        err,
                                        hybridTimestamp(val));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ptupitsyn ptupitsyn merged commit 1223258 into apache:main Mar 31, 2026
5 checks passed
@ptupitsyn ptupitsyn deleted the ignite-28345 branch March 31, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants