From a9008ef1129b679ed4c59fc5023c5ea3fa384799 Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Thu, 17 Sep 2026 15:25:09 +0200 Subject: [PATCH] Fix the inflight correction being skipped by a type mix `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 --- tests/unit/utils/S3UtilsMongoClient.js | 74 +++++++++++++++++++++++++- utils/S3UtilsMongoClient.js | 14 +++-- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/tests/unit/utils/S3UtilsMongoClient.js b/tests/unit/utils/S3UtilsMongoClient.js index 01b5fa77..eb9bf830 100644 --- a/tests/unit/utils/S3UtilsMongoClient.js +++ b/tests/unit/utils/S3UtilsMongoClient.js @@ -5,6 +5,7 @@ const assert = require('assert'); const werelogs = require('werelogs'); const { BucketInfo, ObjectMD, ObjectMDArchive } = require('arsenal').models; const { MongoMemoryReplSet } = require('mongodb-memory-server'); +const { Long } = require('mongodb'); const { constants, errors } = require('arsenal'); const S3UtilsMongoClient = require('../../../utils/S3UtilsMongoClient'); const { @@ -18,6 +19,7 @@ const { const logger = new werelogs.Logger('S3UtilsMongoClient', 'debug', 'debug'); const USERSBUCKET = '__usersbucket'; +const INFOSTORE = '__infostore'; const mongoTestClient = new S3UtilsMongoClient({}); @@ -3127,13 +3129,13 @@ describe('S3UtilsMongoClient, update inflight deltas', () => { { _id: 'bucket_bucket1_1715849127256', usedCapacity: { - _inflight: 3000n, + _inflight: 3000, }, }, { _id: 'bucket_bucket2_1715849127257', usedCapacity: { - _inflight: 5000n, + _inflight: 5000, }, }, ], @@ -3151,6 +3153,74 @@ describe('S3UtilsMongoClient, update inflight deltas', () => { // as they belong to this account: 2000 + 2900 + 3500 assert.strictEqual(output[2].usedCapacity.current, 8400n); }); + + // `updateInflightDeltas` mutates the entries it is given, so build a fresh set + // rather than sharing `metrics` with the tests above. + const longMetrics = () => [ + { + _id: 'bucket_bucket1_1715849127256', + accountOwnerID: '1234', + usedCapacity: { current: 1000n, _inflightsPreScan: 100n }, + }, + { + _id: 'bucket_bucket2_1715849127257', + accountOwnerID: '1234', + usedCapacity: { current: 1000n, _inflightsPreScan: 1500n }, + }, + { + _id: 'account_1234', + usedCapacity: { current: 2000n }, + }, + ].map(entry => S3UtilsMongoClient.convertNumberToLong(entry)); + + it('should compute the inflights deltas with the types mongodb returns', async () => { + // `__infostore` stores BSON Longs and the driver promotes them to Numbers on + // read, while the metrics have already been through `convertNumberToLong` by + // the time this runs: neither side of the arithmetic is a BigInt in production. + const collection = await client.getCollection(INFOSTORE); + await collection.deleteMany({}); + await collection.insertMany([ + { + _id: 'bucket_bucket1_1715849127256', + usedCapacity: { _inflight: Long.fromNumber(3000) }, + }, + { + _id: 'bucket_bucket2_1715849127257', + usedCapacity: { _inflight: Long.fromNumber(5000) }, + }, + ]); + + const output = await client.updateInflightDeltas(longMetrics(), logger); + + // first bucket: 1000 current + (3000 post scan - 100 pre scan) = 3900 + assert.strictEqual(BigInt(output[0].usedCapacity.current), 3900n); + // second bucket: 1000 current + (5000 post scan - 1500 pre scan) = 4500 + assert.strictEqual(BigInt(output[1].usedCapacity.current), 4500n); + // account: 2000 current + 2900 + 3500 = 8400 + assert.strictEqual(BigInt(output[2].usedCapacity.current), 8400n); + }); + + it('should compute the inflights delta when there was no pre scan value', async () => { + // `_inflightsPreScan` is only stored when non-zero, so a bucket which only + // gained inflights during the scan carries no pre scan value at all. + const collection = await client.getCollection(INFOSTORE); + await collection.deleteMany({}); + await collection.insertOne({ + _id: 'bucket_bucket1_1715849127256', + usedCapacity: { _inflight: Long.fromNumber(3000) }, + }); + + const output = await client.updateInflightDeltas([ + S3UtilsMongoClient.convertNumberToLong({ + _id: 'bucket_bucket1_1715849127256', + accountOwnerID: '1234', + usedCapacity: { current: 1000n }, + }), + ], logger); + + // 1000 current + (3000 post scan - 0 pre scan) = 4000 + assert.strictEqual(BigInt(output[0].usedCapacity.current), 4000n); + }); }); describe('S3UtilsMongoClient, cold object helpers', () => { diff --git a/utils/S3UtilsMongoClient.js b/utils/S3UtilsMongoClient.js index 3934a0b9..91a0c7bb 100644 --- a/utils/S3UtilsMongoClient.js +++ b/utils/S3UtilsMongoClient.js @@ -16,7 +16,7 @@ const __COUNT_ITEMS = 'countitems'; const BigIntMax = (...args) => args.reduce((max, current) => { const maxAsBigInt = BigInt(max); const currentAsBigInt = BigInt(current); - return maxAsBigInt > currentAsBigInt ? max : current; + return maxAsBigInt > currentAsBigInt ? maxAsBigInt : currentAsBigInt; }); const baseMetricsObject = { @@ -74,7 +74,10 @@ class S3UtilsMongoClient extends MongoClientInterface { // convert inflights to a map with _id: usedCapacity._inflight const inflightsMap = {}; for (const inflight of inflights) { - const inflightValue = inflight.usedCapacity?._inflight || 0n; + // `_inflight` is stored as a BSON Long and the driver promotes it to a + // Number on read, so it has to be converted before being mixed with + // the BigInt metrics below. + const inflightValue = BigInt(inflight.usedCapacity?._inflight ?? 0); inflightsMap[inflight._id] = inflightValue; } @@ -83,7 +86,12 @@ class S3UtilsMongoClient extends MongoClientInterface { const id = entry._id; if (id.startsWith('bucket_')) { const inflightDocument = inflightsMap[id]; - const inflight = inflightDocument ? BigIntMax(0n, inflightDocument - entry.usedCapacity._inflightsPreScan) : 0n; + // The metrics have been through `convertNumberToLong`, and the + // field is only stored when non-zero. + const preScan = BigInt(entry.usedCapacity._inflightsPreScan ?? 0); + const inflight = inflightDocument + ? BigIntMax(0n, inflightDocument - preScan) + : 0n; if (inflight) { // Inflights remaining after the scan are part of the "current" bytes, // and stored in _inflightsDelta