Skip to content

IGNITE-28271 Use the generated marshalling for GridJobExecuteResponse - #13435

Merged
anton-vinogradov merged 1 commit into
apache:masterfrom
anton-vinogradov:ignite-28271
Aug 5, 2026
Merged

IGNITE-28271 Use the generated marshalling for GridJobExecuteResponse#13435
anton-vinogradov merged 1 commit into
apache:masterfrom
anton-vinogradov:ignite-28271

Conversation

@anton-vinogradov

@anton-vinogradov anton-vinogradov commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

IGNITE-28271

GridJobExecuteResponse carried three hand-written pairs of "object + bytes" — the job exception, the job result and the job attributes — served by marshallUserData and unmarshallUserData, which the callers invoked with an explicit Marshaller. All three become normal @Marshalled fields, and the message becomes a DeferredUnmarshalMessage, the way GridJobExecuteRequest works since IGNITE-28356.

Not through ErrorMessage, and this is the point

The summary of the ticket asks to send the exception as an ErrorMessage. That is what breaks it, and it is very likely why the earlier attempt (IGNITE-26817) was reverted in IGNITE-26890.

ErrorMessage never fails: when it cannot write or read its payload it substitutes a wrapper. GridP2PComputeExceptionTest pins the opposite contract — an exception that cannot be deserialized must reach the caller as a BinaryObjectException, not as a wrapped IgniteException. With ErrorMessage two of its four tests fail.

A plain @Marshalled("gridExBytes") IgniteException gridEx reaches the goal of the ticket — no hand-written marshalling — while the generated code does the same U.unmarshal the old code did, so the failure semantics are untouched.

Worth noting for the record: the revert was not about the peer-deployment class loader. The two tests that cover a peer-deployed exception class pass here, because the message is deferred.

Where the failure policy went

marshallUserData was not only marshalling: a payload that could not be serialized was turned into an exception for the caller, so a job never failed silently. That stays, in the same place and the same shape — before the send, one message going out. Only the marshalling under it is the generated one now.

GridJobProcessor builds a response that carries nothing but the exception, so there is only one thing that can fail there:

if (!loc) {
    try {
        MessageMarshalling.marshal(jobRes, ctx, null);
    }
    catch (IgniteCheckedException e) {
        // The exception is the only payload of this response, so it is what could not be written.
        ...
        jobRes = jobRes.withError(new IgniteException(errMsg));

        MessageMarshalling.marshal(jobRes, ctx, null);
    }
}

GridJobWorker also carries the job result and the job attributes, so it drops the payload first and keeps the job exception — a broken result should not hide why the job actually failed — and only substitutes the exception when that too cannot be written. This is what wrapSerializationError did.

The message keeps one pure method for this, withError(IgniteException), which copies the identity fields and drops the payload. No kernal context, no logger, no marshalling on the message any more.

Pre-marshalling does not double the work: the generated code writes a @Marshalled field only when its companion is still empty (if (obj != null && bytes == null)), so the later pass over the GridIoMessage wrapper finds nothing to do. The old code relied on the same thing.

One thing is not preserved: a second failure is no longer chained onto the first with addSuppressed when both the result and the attributes are broken.

res and jobAttrs get @GridToStringExclude, as in GridJobExecuteRequest — the old code erased the objects after marshalling, the generated code keeps them, and without the exclusion toString() would print a user payload into the log.

The marsh field of GridJobWorker and GridTaskWorker is unused after this and is removed.

Wire format

This changes it: the three payloads keep their @Order slots and their bytes, but the message is no longer unmarshalled by the generic inbound pass. It has to be merged before 2.19 is released.

Checks

  • GridP2PComputeExceptionTest - 4 of 4. This is the suite the earlier attempt was reverted over;
  • GridP2PTimeoutSelfTest - 8 of 8, GridP2PMissedResourceCacheSizeSelfTest - 8 of 8;
  • GridJobStealingSelfTest - 11, GridTaskFailoverSelfTest, IgniteCoreMessagesSerializationTest - green;
  • checkstyle with -Pcheckstyle - no violations.

🤖 Generated with Claude Code

@anton-vinogradov
anton-vinogradov force-pushed the ignite-28271 branch 2 times, most recently from e90485e to fe1eab9 Compare August 4, 2026 22:02
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@anton-vinogradov

anton-vinogradov commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/runall


🚀 RunAll queuedbuild 9254157 · live progress & verdict: Ignite PR Checker. The verdict lands here when the run finishes.
🏁 Run finished — the verdict comment has the full story.

@anton-vinogradov

anton-vinogradov commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Ignite PR Checker verdict · RunAll build 9254157 · 147 suites ran, 0 reused

No blockers — nothing in this run looks caused by this PR. 56 pre-existing/flaky tests filtered out.

♻️ Settled after 1 auto re-run wave(s): #1 — 2 blocker suite(s).

@anton-vinogradov
anton-vinogradov merged commit 5cbb7fc into apache:master Aug 5, 2026
7 checks passed
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.

2 participants