Skip to content

Commit

Permalink
bf: S3UTILS-180 unbounded number of sproxyd keys in Map
Browse files Browse the repository at this point in the history
- Fix missing transmission of the environment variable that limits the
  number of keys kept in the in-memory maps
  (`OBJECT_REPAIR_DUPLICATE_KEYS_WINDOW_SIZE`)

- Add `Number.parseInt()` calls to environment variables where needed

- Sanitize a little bit the `env.js`:

  - reorder lexicographically

  - set default value of `OBJECT_REPAIR_RAFT_LOG_BEGIN_CSEQ` to `null`
    to make ObjectRepair use the computed default when fetched
    (instead of having it declared separately from `env.js`)

Manually tested with a mocked oplog server returning entries with
sproxyd keys:

- without the fix: memory usage grows quickly (multiple GB after a few
  minutes)

- with the fix: memory stabilizes at a level proportional to the value
  of `OBJECT_REPAIR_DUPLICATE_KEYS_WINDOW_SIZE`

- not specifying `OBJECT_REPAIR_DUPLICATE_KEYS_WINDOW_SIZE` in the
  environment: stabilizes memory usage around the default value of
  100000 entries.
  • Loading branch information
jonathan-gramain committed Sep 23, 2024
1 parent c680ebf commit 44f59b0
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 14 deletions.
2 changes: 1 addition & 1 deletion ObjectRepair/SproxydKeysSubscribers.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class DuplicateSproxydKeyFoundHandler {
this._getObjectURL = getObjectURL;
this.queue = queue(this._repairObject, 1);
// use OBJECT_REPAIR_DUPLICATE_KEYS_WINDOW_SIZE since there will be at least one sproxyd key per object
this.visitedObjects = new BoundedMap(env.DUPLICATE_KEYS_WINDOW_SIZE);
this.visitedObjects = new BoundedMap(env.OBJECT_REPAIR_DUPLICATE_KEYS_WINDOW_SIZE);
}

/**
Expand Down
20 changes: 14 additions & 6 deletions ObjectRepair/env.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
const env = {
OBJECT_REPAIR_BUCKETD_HOSTPORT: process.env.OBJECT_REPAIR_BUCKETD_HOSTPORT,
OBJECT_REPAIR_SPROXYD_HOSTPORT: process.env.OBJECT_REPAIR_SPROXYD_HOSTPORT,
OBJECT_REPAIR_RAFT_SESSION_ID: process.env.OBJECT_REPAIR_RAFT_SESSION_ID,
OBJECT_REPAIR_RAFT_LOG_BATCH_SIZE: process.env.OBJECT_REPAIR_RAFT_LOG_BATCH_SIZE || 1000,
OBJECT_REPAIR_LOOKBACK_WINDOW: process.env.OBJECT_REPAIR_LOOKBACK_WINDOW || 10000,
OBJECT_REPAIR_LOG_LEVEL: process.env.OBJECT_REPAIR_LOG_LEVEL || 'info',
OBJECT_REPAIR_DUMP_LEVEL: process.env.OBJECT_REPAIR_DUMP_LEVEL || 'error',
OBJECT_REPAIR_LOG_INTERVAL: process.env.OBJECT_REPAIR_LOG_INTERVAL || 300,
OBJECT_REPAIR_DUPLICATE_KEYS_WINDOW_SIZE:
Number.parseInt(process.env.OBJECT_REPAIR_DUPLICATE_KEYS_WINDOW_SIZE, 10) || 100000,
OBJECT_REPAIR_LOG_INTERVAL:
Number.parseInt(process.env.OBJECT_REPAIR_LOG_INTERVAL, 10) || 300,
OBJECT_REPAIR_LOG_LEVEL: process.env.OBJECT_REPAIR_LOG_LEVEL || 'info',
OBJECT_REPAIR_LOOKBACK_WINDOW:
Number.parseInt(process.env.OBJECT_REPAIR_LOOKBACK_WINDOW, 10) || 10000,
OBJECT_REPAIR_RAFT_LOG_BATCH_SIZE:
Number.parseInt(process.env.OBJECT_REPAIR_RAFT_LOG_BATCH_SIZE, 10) || 1000,
OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ:
Number.parseInt(process.env.OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ, 10) || null,
OBJECT_REPAIR_RAFT_SESSION_ID:
Number.parseInt(process.env.OBJECT_REPAIR_RAFT_SESSION_ID, 10) || undefined,
OBJECT_REPAIR_SPROXYD_HOSTPORT: process.env.OBJECT_REPAIR_SPROXYD_HOSTPORT,
};

module.exports = { env };
12 changes: 5 additions & 7 deletions ObjectRepair/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,26 @@ Optional environment variables:
OBJECT_REPAIR_TLS_CA_PATH: path to TLS alternate CA file
`;
for (const [key, value] of Object.entries(env)) {
if (!value) {
if (value === undefined) {
log.info(`${key} must be defined`);
console.error(USAGE);
process.exit(1);
}
}

env.OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ = process.env.OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ;

/**
* Creates new reader and runs until stop().
* @returns {undefined}
*/
function runJournalReader() {
if (env.OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ === undefined) {
if (env.OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ === null) {
log.info('OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ is not defined.'
+ 'Ingestion will start at latest cseq - OBJECT_REPAIR_LOOKBACK_WINDOW');
}
const reader = new RaftJournalReader(
Number.parseInt(env.OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ, 10),
Number.parseInt(env.OBJECT_REPAIR_RAFT_LOG_BATCH_SIZE, 10),
Number.parseInt(env.OBJECT_REPAIR_RAFT_SESSION_ID, 10),
env.OBJECT_REPAIR_RAFT_LOG_BEGIN_SEQ,
env.OBJECT_REPAIR_RAFT_LOG_BATCH_SIZE,
env.OBJECT_REPAIR_RAFT_SESSION_ID,
);
reader.run();
}
Expand Down

0 comments on commit 44f59b0

Please sign in to comment.