[
https://issues.apache.org/jira/browse/IGNITE-28931?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anton Vinogradov updated IGNITE-28931:
--------------------------------------
Labels: IEP-132 wire-format (was: IEP-132)
> Remove the dead entry processor machinery from GridDhtAtomicUpdateRequest
> -------------------------------------------------------------------------
>
> Key: IGNITE-28931
> URL: https://issues.apache.org/jira/browse/IGNITE-28931
> Project: Ignite
> Issue Type: Task
> Reporter: Anton Vinogradov
> Priority: Major
> Labels: IEP-132, wire-format
> Fix For: 2.19
>
>
> GridDhtAtomicUpdateRequest still carries the forceTransformBackups flag and
> everything gated by it, but
> the flag has been false on every path since 2015. Both places that build the
> request pass the literal:
> GridDhtAtomicUpdateFuture:83-95 invokeArgs = null,
> forceTransformBackups = false
> GridDhtAtomicSingleUpdateFuture:99-111 invokeArgs = null,
> forceTransformBackups = false
> There is no third producer - `grep -rn "new GridDhtAtomicUpdateRequest"`
> returns exactly these two lines,
> and no test builds the request.
> GridDhtAtomicSingleUpdateRequest#forceTransformBackups returns the literal
> false as well, so every consumer of the flag is constant:
> GridDhtAtomicCache:3307 intercept = req.forceTransformBackups() && ...
> -> always false
> GridDhtAtomicCache:3357 /*check version*/ !req.forceTransformBackups()
> -> always true
> GridNearAtomicCache:322, :374
> -> same
> Confirmed on a live cluster: three nodes, PARTITIONED/ATOMIC with backups and
> a near cache and
> write-through, invokeAll and putAll driven from every node. All 36 recorded
> GridDhtAtomicUpdateRequest
> instances had forceTransformBackups == false, invokeArguments() == null, and
> a null entry processor at
> every index, near indexes included.
> What this costs today: four wire fields on every DHT backup request that is
> not the single-key variant -
> @Order(11) forceTransformBackups, @Order(12) entryProcessorsBytes, @Order(13)
> nearEntryProcessorsBytes,
> @Order(14) invokeArgsBytes. A boolean is one byte and a null collection is
> writeInt(-1), which is one byte
> in the varint encoding, so removing them saves exactly four bytes per request.
> The branch also holds a latent defect that shows it was never exercised: the
> eager
> `entryProcessorsBytes = new ArrayList<>()` in the constructor (:184) and in
> addNearWriteValue (:284) sit
> inside `if (forceTransformBackups)`, while marshal (:481, :484) and deploy
> (:513, :516) gate on `== null`.
> Had the flag ever been true, the processors would never have been marshalled
> and an empty list would have
> reached the backup. That gate arrived in 457a9ae4d3b (2016-01), ten months
> after the branch died in
> 3381630e7b1 (2015-03, ignite-629), which is why it never fired.
> Scope of the removal: the four wire fields and their logical counterparts,
> the two constructor parameters
> and the assert, the entryProc branches of addWriteValue and
> addNearWriteValue, marshal/unmarshal/deploy
> and the MarshallableMessage and DeployableMessage interfaces, the abstract
> accessors in
> GridDhtAtomicAbstractUpdateRequest together with the
> GridDhtAtomicSingleUpdateRequest overrides, and the
> constant consumers listed above folded to their constants.
> GridCacheMessage#marshallCollection and
> #unmarshalCollection are orphaned by this and can go too; the other helpers
> stay, they serve
> GridNearAtomicFullUpdateRequest.
> Note that the entryProc parameter of addWriteValue is not always null:
> GridDhtAtomicCache:2863-2864
> (updateWithBatch, more than one key, write-through, no DR) passes a real
> processor, and
> GridDhtAtomicUpdateRequest drops it silently at :205-211 because the computed
> value is what goes to the
> backup. The single-key request asserts this instead. So the parameter is
> removed because the request
> ignores it, not because callers never pass one.
> Removing wire fields changes the message layout, so nodes of mixed versions
> cannot read each other -
> worth calling out for rolling upgrades.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
