[
https://issues.apache.org/jira/browse/IGNITE-26630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18102127#comment-18102127
]
Anton Vinogradov commented on IGNITE-26630:
-------------------------------------------
The investigation is done and written into the description. Short version:
partition() is a stripe key, not a data partition, so it cannot be removed —
but six messages carry a field on the wire only to pick a stripe, and that part
is a wire change, so it has to land before 2.19. Three similar looking fields
must stay, they are listed as out of scope. Taking this.
> 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
>
>
> 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
> Six messages 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}} | {{partition()}} is
> final, subclasses pass the value to super |
> | GridDhtAtomicNearResponse | {{@Order(0) partId}} | |
> | GridDhtAtomicUpdateResponse | {{@Order(3) partId}} | |
> | GridNearAtomicUpdateResponse | {{@Order(5) partId}} | the setter
> {{partition(int)}} at :365 has no callers at all |
> | DataStreamerRequest | {{@Order(15) partId}} | the value is the sender
> stripe index, not a data partition; DataStreamerImpl.java:2005 sends
> {{STRIPE_DISABLED_PART}} unless the receiver is the isolated updater |
> 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.
> 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)
