fix: CountItems inflight correction skipped by a BigInt/Number type mix - #407
Conversation
`updateInflightDeltas` is reached after `convertNumberToLong` has turned
every metric into a BSON Long, and `_inflight` is read back from MongoDB
as a Number because `promoteLongs` defaults to true. Neither side of the
subtraction is a BigInt, so `BigIntMax` -- which compares as BigInt but
returned the unconverted operand -- handed a Number to
`BigInt(current) + inflight`, raising:
TypeError: Cannot mix BigInt and other types, use explicit conversions
`_inflightsPreScan` is also only stored when non-zero, so a bucket which
only gained inflights during the scan produced `NaN` and a RangeError
from `BigInt(NaN)` instead.
Both are caught and logged, and the function then returns the metrics
unchanged, so the correction was applied to no bucket and no account:
`usedCapacity.current` silently excluded the inflights remaining after
the scan, and `_inflightsDelta` was never written.
Convert at the two boundaries and make `BigIntMax` return its converted
operands so it cannot launder a type again.
The existing unit test stubbed the collection with `_inflight: 3000n`, a
value MongoDB cannot return, which is why this was never caught; it now
uses a Number, and two tests exercise the real in-memory MongoDB so the
driver's own conversions are covered.
Issue: S3UTILS-245
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development/1.17 #407 +/- ##
====================================================
+ Coverage 45.28% 45.32% +0.04%
====================================================
Files 88 88
Lines 6486 6489 +3
Branches 1360 1363 +3
====================================================
+ Hits 2937 2941 +4
+ Misses 3503 3502 -1
Partials 46 46 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/approve |
|
!done 2h |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
This pull request did not target the following hotfix branch(es) so they
Please check the status of the associated issue S3UTILS-245. Goodbye delthas. The following options are set: approve |
|
|
updateInflightDeltasthrows on every completedcount-itemsrun on any platform with concurrent writes, and the inflight correction is then silently skipped for every bucket and account.{"method":"updateInflightDeltas","errorString":"TypeError: Cannot mix BigInt and other types, use explicit conversions"}Observed on a customer platform in two sosreports three months apart, one occurrence per completed run. An idle platform shows zero — consistent with the trigger.
Mechanism
:963—convertNumberToLong(metrics)turns everybigintinto a BSONLong, so by the timeupdateInflightDeltasruns at:969bothusedCapacity.currentand_inflightsPreScanareLong.:86—_inflightcomes from MongoDB, wherepromoteLongsdefaults totrue, so it is a plainNumber. The subtraction is Number arithmetic and yields a Number.:19—BigIntMaxconverts both operands to compare them, then returns the unconverted one, so a Number comes back out.:91—BigInt(current) + <Number>→ TypeError.Second path:
_inflightsPreScanis only stored when non-zero (:414), so a bucket that only gained inflights during the scan gives5000 - undefined=NaN→BigInt(NaN)→RangeError.Both are caught at
:122, logged once, andallMetricsis returned unmodified.Fix
Convert at the two boundaries, and make
BigIntMaxreturn its converted operands so it cannot launder a type again. The correct pattern already existed at:1035, which reads the same field asBigInt(doc.usedCapacity._inflight.toString()).The other ten
BigIntMaxcall sites (:690-707,:725,:728) already pass BigInt values against0n, so that change is a no-op for them.Why it was never caught
The unit test stubbed the collection with
_inflight: 3000n— a BigInt literal MongoDB cannot return, since nothing setsuseBigInt64. With BigInt on both sides the arithmetic succeeded and the test passed.That stub now uses a
Number, and two new tests drive the real in-memory MongoDB without stubbinggetCollection, so the driver's own conversions are exercised. Both fail on the parent commit withExpected 3900n, Received 1000n— the metric left uncorrected — and pass here.Regression range
Correct when added (
490de27, S3UTILS-163, 1.14.8) — it was written in defensive Number arithmetic. The BigInt migration (1460431, 1.14.18) replacedMath.maxwithBigIntMaxand dropped theNumber()/Long.fromNumber()coercions. Affected from 1.14.18 through 1.19.1.Verification
yarn test:unitlocally: 32 suites, 456 tests, all passing.eslintclean.Issue: S3UTILS-245