Anton Vinogradov created IGNITE-28931:
-----------------------------------------
Summary: 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
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)
