[
https://issues.apache.org/jira/browse/IGNITE-26630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18102152#comment-18102152
]
Anton Vinogradov commented on IGNITE-26630:
-------------------------------------------
Correction to the scope above: {{DataStreamerRequest#partId}} is not one of the
fields step 2 can drop. Unlike the five responses, it does not echo a request —
it carries the decision whether to stripe at all (DataStreamerImpl.java:2005),
and the receiver cannot take that decision itself, because the stripe is picked
before the payload is read. Step 2 covers five fields, not six.
> 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: 20m
> 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)
