Merge remote-tracking branch 'origin/bugfix/ARSN-489' into w/8.2/bugfix/ARSN-489

Author: williamlardier

lib/storage/metadata/mongoclient/MongoClientInterface.ts

@@ -37,7 +37,7 @@ import {
 } from 'mongodb';
 import { v4 as uuidv4 } from 'uuid';
 
-import { generateVersionId as genVID } from '../../../versioning/VersionID';
+import { generateUniqueVersionId } from '../../../versioning/VersionID';
 import * as listAlgos from '../../../algos/list/exportAlgos';
 import LRUCache from '../../../algos/cache/LRUCache';
 
@@ -75,8 +75,6 @@ const CONCURRENT_CURSORS = process.env.CONCURRENT_CURSORS ?
 
 const initialInstanceID = process.env.INITIAL_INSTANCE_ID;
 
-let uidCounter = 0;
-
 const BUCKET_VERSIONS = require('../../../versioning/constants')
     .VersioningConstants.BucketVersioningKeyFormat;
 const DEFAULT_BUCKET_KEY_FORMAT =
@@ -87,12 +85,6 @@ const DEFAULT_BUCKET_KEY_FORMAT =
 const DB_PREFIXES = require('../../../versioning/constants')
     .VersioningConstants.DbPrefixes;
 
-function generateVersionId(replicationGroupId) {
-    // generate a unique number for each member of the nodejs cluster
-    return genVID(`${process.pid}.${uidCounter++}`,
-        replicationGroupId);
-}
-
 function inc(str) {
     return str ? (str.slice(0, str.length - 1) +
         String.fromCharCode(str.charCodeAt(str.length - 1) + 1)) : str;
@@ -845,7 +837,7 @@ class MongoClientInterface {
         cb: ArsenalCallback<string>,
         isRetry?: boolean,
     ) {
-        const versionId = generateVersionId(this.replicationGroupId);
+        const versionId = generateUniqueVersionId(this.replicationGroupId);
         objVal.versionId = versionId;
         const versionKey = formatVersionKey(objName, versionId, params.vFormat);
         const masterKey = formatMasterKey(objName, params.vFormat);
@@ -972,7 +964,7 @@ class MongoClientInterface {
         log: werelogs.Logger,
         cb: ArsenalCallback<string>,
     ) {
-        const versionId = generateVersionId(this.replicationGroupId);
+        const versionId = generateUniqueVersionId(this.replicationGroupId);
         objVal.versionId = versionId;
         const masterKey = formatMasterKey(objName, params.vFormat);
         c.updateOne({ _id: masterKey },
@@ -1803,7 +1795,7 @@ class MongoClientInterface {
     ) {
         const masterKey = formatMasterKey(objName, params.vFormat);
         const versionKey = formatVersionKey(objName, params.versionId, params.vFormat);
-        const _vid = generateVersionId(this.replicationGroupId);
+        const _vid = generateUniqueVersionId(this.replicationGroupId);
         async.series([
             next => c.updateOne(
                 {

lib/versioning/VersionID.ts

@@ -23,6 +23,10 @@ const TEMPLATE_RG = new Array(LENGTH_RG + 1).join(' ');
 
 export const S3_VERSION_ID_ENCODING_TYPE = process.env.S3_VERSION_ID_ENCODING_TYPE;
 
+// Counter that is increased after each call to generateUniqueVersionId
+export let uidCounter = 0;
+export const versionIdSeed = getVersionIdSeed();
+
 /**
  * Left-pad a string representation of a value with a given template.
  * For example: pad('foo', '00000') gives '00foo'.
@@ -87,6 +91,23 @@ function wait(span: number) {
     }
 }
 
+export function getVersionIdSeed(): string {
+    // The HOSTNAME environment variable is set by default by Kubernetes
+    // and populated with the pod name, containing a suffix with a unique id
+    // as a string.
+    // By default, we rely on the pid, to account for multiple workers in
+    // cluster mode. As a result, the unique id is either <pod-suffix>.<pid>
+    // or <pid>.
+    // If unique vID are needed in a multi cluster mode architecture (i.e.,
+    // multiple server instances, each with multiple workers), the
+    // HOSTNAME environment variable can be set.
+    return `${process.env.HOSTNAME?.split('-').pop() || ''}${process.pid}`;
+}
+
+export function generateUniqueVersionId(replicationGroupId: string): string {
+    return generateVersionId(`${versionIdSeed}.${uidCounter++}`, replicationGroupId);
+}
+
 /**
  * This function returns a "versionId" string indicating the current time as a
  * combination of the current time in millisecond, the position of the request

tests/unit/versioning/VersionID.spec.js

@@ -22,6 +22,29 @@ function generateRandomVIDs(count) {
 const count = 1000000;
 
 describe('test generating versionIds', () => {
+    describe('getVersionIdSeed', () => {
+        it('should return the correct versionIdSeed', () => {
+            const versionIdSeed = VID.getVersionIdSeed();
+            assert.strictEqual(versionIdSeed, process.pid.toString());
+        });
+
+        it('should return the correct versionIdSeed when HOSTNAME is set', () => {
+            process.env.HOSTNAME = 'test-pod-123';
+            const versionIdSeed = VID.getVersionIdSeed();
+            assert.strictEqual(versionIdSeed.startsWith('123'), true);
+        });
+    });
+
+    describe('generateUniqueVersionId', () => {
+        it('should increase the uidCounter', () => {
+            const versionId1 = VID.generateUniqueVersionId('somestring');
+            const versionId2 = VID.generateUniqueVersionId('somestring');
+            assert.notStrictEqual(versionId1, versionId2);
+            assert(VID.uidCounter > 0);
+            assert(VID.versionIdSeed);
+        });
+    });
+
     describe('invalid IDs', () => {
         // A client can use the CLI to send requests with arbitrary version IDs.
         // These IDs may contain invalid characters and should be handled gracefully.