[
https://issues.apache.org/jira/browse/IGNITE-26630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18102265#comment-18102265
]
Anton Vinogradov commented on IGNITE-26630:
-------------------------------------------
[Ignite PR Checker|https://ignite-pr-checker.is-a.dev/?pr=13437] verdict for PR
13437 · RunAll build [9258046|https://ci2.ignite.apache.org/build/9258046] ·
147 suites ran, 0 reused
(!) *This run doesn't cover the PR fully:*
- 2 suite(s) have no reliable result (compilation error, timeout, crash)
(!) *2 broken suite(s)* (failed without a reliable run):
- Snapshots 6: JVM crash / out of memory · 1 failed test detected
- Continuous Query 1: execution timeout · non-zero exit code · Number of tests
61 is 72% less than 216 in build #42270 — ran 61 of master's 216 tests
(x) *3 blocker(s) in 3 suite(s):*
- Platform .NET (Core Linux): {{Apache.Ignite.Core.Tests.DotNetCore:
Apache.Ignite.Core.Tests.Compute.IgniteExceptionTaskSelfTest.TestMapNotMarshalableJob}}
- Snapshots 1: {{org.apache.ignite.testsuites.IgniteSnapshotTestSuite:
org.apache.ignite.internal.processors.cache.persistence.snapshot.IgniteClusterSnapshotSelfTest.testClientHandlesSnapshotFailOnStartStage[encryption=true,
onlyPrimay=false]}}
- Platform .NET (Windows) 3: {{Apache.Ignite.Core.Tests.exe:
Apache.Ignite.Core.Tests.Client.Datastream.DataStreamerClientTest.TestBinaryStreamerCreatesSqlRecord}}
⏳ _Auto re-run *#1* in progress — 3 blocker + 2 broken suite(s) re-queued
(attempt 1/2), *≈ settled by 03:54 MSK*. This comment updates when they settle._
> Investigate removal of GridCacheMessage#partition
> -------------------------------------------------
>
> Key: IGNITE-26630
> URL: https://issues.apache.org/jira/browse/IGNITE-26630
> Project: Ignite
> Issue Type: Task
> Reporter: Ilya Shishkov
> Assignee: Anton Vinogradov
> Priority: Minor
> Labels: IEP-132, ise, wire-format
> Fix For: 2.19
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> h3. What partition() actually is
> *{{GridCacheMessage#partition()}}* is not a data partition. It is the key
> that picks a stripe: {{GridIoMessage#partition()}} reads it
> (GridIoMessage.java:183-190), and {{GridIoManager}} hands the message to the
> striped executor when the value is not {{STRIPE_DISABLED_PART}}
> (GridIoManager.java:1392-1401).
> So the method cannot simply be removed. The base class returns {{-1}}
> (GridCacheMessage.java:129) and {{-1}} is not {{STRIPE_DISABLED_PART}}
> ({{Integer.MIN_VALUE}}, GridIoMessage.java:38), so even a message that does
> not care goes to the striped pool. Removing the method would move every cache
> message to the plain system pool. Functional tests would not notice; only a
> benchmark would.
> h3. Step 1: no wire change
> Replace the {{instanceof}} chain in {{GridIoMessage#partition()}} with an
> interface, and name the method for what it is — a stripe key, not a
> partition. The chain covers {{GridCacheMessage}} and {{DataStreamerRequest}}
> today and returns {{STRIPE_DISABLED_PART}} for everything else.
> h3. Step 2: wire change, so it must land before 2.19
> Five responses carry a field on the wire only to pick a stripe. A response
> echoes the partition of its request so that it lands on the same stripe, and
> nothing on the receiving side reads the value.
> || class || field || note ||
> | GridDistributedTxPrepareResponse | {{@Order(1) part}} | |
> | GridDistributedTxFinishResponse | {{@Order(2) part}} | {{stripeIdx()}} is
> final, subclasses pass the value to super |
> | GridDhtAtomicNearResponse | {{@Order(0) partId}} | |
> | GridDhtAtomicUpdateResponse | {{@Order(3) partId}} | |
> | GridNearAtomicUpdateResponse | {{@Order(5) partId}} | |
> Dropping a field means the message is no longer pinned to the stripe of its
> request. Nothing reads the value, but work moves between stripes, so each
> class needs its own answer to "is losing that pinning safe here". This step
> needs a benchmark: tests stay green either way.
> {{DataStreamerRequest}} {{@Order(15) partId}} looks like a sixth candidate
> but is not one. It is not an echo: DataStreamerImpl.java:2005 puts either the
> stripe or {{NO_STRIPE}} there, depending on the receiver, so the value
> carries a decision. The receiver cannot make that decision itself, because
> the stripe is picked before the payload is read — the receiver identity lives
> in {{updaterBytes}} and unmarshalling it on the NIO thread is exactly what
> should not happen. Removing this field needs a separate answer.
> h3. Out of scope
> Three fields look the same and must stay:
> * {{GridCacheQueryRequest}} {{@Order(18) part}} — a real scan partition. It
> comes from the user and is read at GridCacheDistributedQueryManager.java:251.
> * {{GridNearAtomicCheckUpdateRequest}} {{@Order(0) partId}} — read at
> GridDhtAtomicCache.java:3244 and copied into the response; the primary cannot
> restore it from anything else.
> * {{GridJobExecuteRequest}} {{@Order(22) part}} — the class is not a
> {{GridCacheMessage}}, so it never reaches the striping chain. The value is an
> affinity partition, read at GridJobProcessor.java:1188.
> Messages that compute the value need no change: {{GridDhtTxPrepareRequest}},
> {{GridDhtTxFinishRequest}}, {{GridNearTxPrepareRequest}} and
> {{GridNearTxFinishRequest}} use {{U.safeAbs(version().hashCode())}};
> {{GridDistributedLockRequest}} and {{GridNearUnlockRequest}} take it from the
> first key; {{GridDhtPartitionsAbstractMessage}} returns
> {{STRIPE_DISABLED_PART}}.
> h3. One thing to keep in mind
> For DHT atomic updates the stripe carries correctness, not only speed: the
> deferred response buffer is a {{ThreadLocal}}
> (GridDhtAtomicCache.java:166-171) and the timeout flushes it back into the
> same stripe (:3444, :3493). The contract is guarded by {{assert
> Thread.currentThread().getName().startsWith("sys-stripe-")}} (:3259) — an
> assert on a thread name, which does nothing unless the JVM runs with {{-ea}}.
> That path is on the request side, where the stripe key is computed from the
> keys, so step 2 does not touch it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
