support new versionId format

Kerkesni · Kerkesni · commit b52a8da65ee9 · 2025-07-01T18:56:27.000+02:00
see https://scality.atlassian.net/wiki/spaces/OS/pages/3225583692/VersionID+Collisions+in+Zenko Issue: ARSN-502
diff --git a/lib/storage/metadata/MetadataWrapper.js b/lib/storage/metadata/MetadataWrapper.js
@@ -117,6 +117,7 @@ class MetadataWrapper {
                 replicaSet: params.mongodb.replicaSet,
                 readPreference: params.mongodb.readPreference,
                 database: params.mongodb.database,
+                instanceId: params.instanceId,
                 replicationGroupId: params.replicationGroupId,
                 path: params.mongodb.path,
                 authCredentials: params.mongodb.authCredentials,
diff --git a/lib/storage/metadata/mongoclient/MongoClientInterface.ts b/lib/storage/metadata/mongoclient/MongoClientInterface.ts
@@ -37,7 +37,7 @@ import {
 } from 'mongodb';
 import { v4 as uuidv4 } from 'uuid';
 
-import { generateUniqueVersionId } from '../../../versioning/VersionID';
+import { generateVersionId } from '../../../versioning/VersionID';
 import * as listAlgos from '../../../algos/list/exportAlgos';
 import LRUCache from '../../../algos/cache/LRUCache';
 
@@ -98,6 +98,7 @@ export type MongoDBClientInterfaceParameters = {
     path: string,
     database: string,
     logger: werelogs.Logger,
+    instanceId: string,
     replicationGroupId: string,
     authCredentials: MongoUtils.AuthCredentials,
     isLocationTransient: Function,
@@ -245,6 +246,7 @@ class MongoClientInterface {
     private client: MongoClient | null;
     private db: Db | null;
     private path: string;
+    private instanceId: string;
     private replicationGroupId: string;
     private database: string;
     private isLocationTransient: Function;
@@ -261,7 +263,7 @@ class MongoClientInterface {
 
     constructor(params: MongoDBClientInterfaceParameters) {
         const { replicaSetHosts, writeConcern, replicaSet, readPreference, path,
-            database, logger, replicationGroupId, authCredentials,
+            database, logger, instanceId, replicationGroupId, authCredentials,
             isLocationTransient, shardCollections } = params;
         const cred = MongoUtils.credPrefix(authCredentials);
         this.mongoUrl = `mongodb://${cred}${replicaSetHosts}/` +
@@ -276,6 +278,7 @@ class MongoClientInterface {
         this.adminDb = null;
         this.logger = logger;
         this.path = path;
+        this.instanceId = instanceId;
         this.replicationGroupId = replicationGroupId;
         this.database = database;
         this.isLocationTransient = isLocationTransient;
@@ -837,7 +840,7 @@ class MongoClientInterface {
         cb: ArsenalCallback<string>,
         isRetry?: boolean,
     ) {
-        const versionId = generateUniqueVersionId(this.replicationGroupId);
+        const versionId = generateVersionId(this.instanceId, this.replicationGroupId);
         objVal.versionId = versionId;
         const versionKey = formatVersionKey(objName, versionId, params.vFormat);
         const masterKey = formatMasterKey(objName, params.vFormat);
@@ -964,7 +967,7 @@ class MongoClientInterface {
         log: werelogs.Logger,
         cb: ArsenalCallback<string>,
     ) {
-        const versionId = generateUniqueVersionId(this.replicationGroupId);
+        const versionId = generateVersionId(this.instanceId, this.replicationGroupId);
         objVal.versionId = versionId;
         const masterKey = formatMasterKey(objName, params.vFormat);
         c.updateOne({ _id: masterKey },
@@ -1795,7 +1798,7 @@ class MongoClientInterface {
     ) {
         const masterKey = formatMasterKey(objName, params.vFormat);
         const versionKey = formatVersionKey(objName, params.versionId, params.vFormat);
-        const _vid = generateUniqueVersionId(this.replicationGroupId);
+        const _vid = generateVersionId(this.instanceId, this.replicationGroupId);
         async.series([
             next => c.updateOne(
                 {
diff --git a/lib/versioning/VersionID.ts b/lib/versioning/VersionID.ts
@@ -1,31 +1,53 @@
-// VersionID format:
+// Hex VersionID format:
 //         timestamp  sequential_position  rep_group_id  other_information
 // where:
 // - timestamp              14 bytes        epoch in ms (good untill 5138)
 // - sequential_position    06 bytes        position in the ms slot (1B ops)
 // - rep_group_id           07 bytes        replication group identifier
 // - other_information      arbitrary       user input, such as a unique string
+//
+// Base62 VersionID format:
+//         timestamp  sequential_position  rep_group_id  instance_id  version_id_format
+// where:
+// - timestamp              14 bytes        epoch in ms
+// - sequential_position    06 bytes        position in the ms slot
+// - rep_group_id           07 bytes        replication group identifier
+// - instance_id            06 bytes        unique instance identifier (optional)
+// - version_id_format      02 bytes        version ID format marker + version
 
 import base62Integer from 'base62';
 import baseX from 'base-x';
+import assert from 'assert';
+import { VersioningConstants } from './constants';
 const BASE62 = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
 const base62String = baseX(BASE62);
 
 // the lengths of the components in bytes
 const LENGTH_TS = 14; // timestamp: epoch in ms
 const LENGTH_SEQ = 6; // position in ms slot
 const LENGTH_RG = 7; // replication group id
+const LENGTH_ID = 6; // instance id, can be empty
+const LENGTH_FT = 2; // version ID format, 1 byte + separator
 
 // empty string template for the variables in a versionId
 const TEMPLATE_TS = new Array(LENGTH_TS + 1).join('0');
 const TEMPLATE_SEQ = new Array(LENGTH_SEQ + 1).join('0');
-const TEMPLATE_RG = new Array(LENGTH_RG + 1).join(' ');
+const TEMPLATE_RG = new Array(LENGTH_RG + 1).join('0');
+const TEMPLATE_ID = new Array(LENGTH_ID + 1).join('0');
 
 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();
+const versionIDFormat = '1';
+
+/**
+ * Check if the S3_VERSION_ID_ENCODING_TYPE is set to 'hex' or not set at all.
+ * If it is, we use the legacy hex encoding for version IDs.
+ *
+ * @return - true if hex encoding type is used, false otherwise
+ */
+export function isHexEncodingType() {
+    return S3_VERSION_ID_ENCODING_TYPE === 'hex' || !S3_VERSION_ID_ENCODING_TYPE;
+}
 
 /**
  * Left-pad a string representation of a value with a given template.
@@ -91,23 +113,6 @@ 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
@@ -124,6 +129,21 @@ export function generateVersionId(info: string, replicationGroupId: string): str
     // replication group ID, like PARIS; will be trimmed if exceed LENGTH_RG
     const repGroupId = padRight(replicationGroupId, TEMPLATE_RG);
 
+    let otherInfo = '';
+    let instanceIdPadded = '';
+    let vidFormat = '';
+
+    if (isHexEncodingType()) {
+        // In HEX encoding, the full info data is used.
+        otherInfo = info;
+    } else {
+        // In base62, info is for the instance ID and is trimmed/padded.
+        instanceIdPadded = padRight(info, TEMPLATE_ID);
+        // versionID format, 2 bytes
+        vidFormat = VersioningConstants.VersionId.FormatMarker + versionIDFormat;
+        assert(vidFormat.length === LENGTH_FT, `versionID format must be ${LENGTH_FT} bytes`);
+    }
+
     // Need to wait for the millisecond slot got "flushed". We wait for
     // only a single millisecond when the module is restarted, which is
     // necessary for the correctness of the system. This is therefore cheap.
@@ -143,13 +163,6 @@ export function generateVersionId(info: string, replicationGroupId: string): str
     lastSeq = lastTimestamp === ts ? lastSeq + 1 : 0;
     lastTimestamp = ts;
 
-    // if S3_VERSION_ID_ENCODING_TYPE is "hex", info is used.
-    if (S3_VERSION_ID_ENCODING_TYPE === 'hex' || !S3_VERSION_ID_ENCODING_TYPE) {
-        // info field stays as is
-    } else {
-        info = '';
-    }
-
     // In the default cases, we reverse the chronological order of the
     // timestamps so that all versions of an object can be retrieved in the
     // reversed chronological order---newest versions first. This is because of
@@ -158,7 +171,9 @@ export function generateVersionId(info: string, replicationGroupId: string): str
         padLeft(MAX_TS - lastTimestamp, TEMPLATE_TS) +
         padLeft(MAX_SEQ - lastSeq, TEMPLATE_SEQ) +
         repGroupId +
-        info
+        otherInfo +
+        instanceIdPadded +
+        vidFormat
     );
 }
 
@@ -271,6 +286,30 @@ export function base62Decode(str: string): string | Error {
 export const ENC_TYPE_HEX = 0; // legacy (large) encoding
 export const ENC_TYPE_BASE62 = 1; // new (tiny) encoding
 
+/**
+ * Checks if the given versionId string contains the specified format version.
+ * For performance, this function assumes the format marker and version are always
+ * located at the end of the versionId (at position versionId.length - LENGTH_FT).
+ * This allows for O(1) access without scanning the entire string.
+ *
+ * @param versionId - The versionId string to check.
+ * @param version - The expected format version.
+ * @returns true if the versionId contains the format marker and version, false otherwise.
+ */
+function hasVersionIDFormat(versionId: string, version: string): boolean {
+    const formatMarkerIdx = versionId.length - LENGTH_FT;
+    const formatMarker = versionId.charAt(formatMarkerIdx);
+    if (formatMarker !== VersioningConstants.VersionId.FormatMarker) {
+        return false; // no format marker
+    }
+    const formatVersion = versionId.substring(formatMarkerIdx + 1);
+    return formatVersion === version;
+}
+
+const LEGACY_BASE62_DECODED_LENGTH = 27;
+const BASE62_DECODED_LENGTH = 35;
+const BASE62_ENCODED_LENGTH = 32;
+
 /**
  * Encode a versionId to obscure internal information contained
  * in a version ID.
@@ -279,8 +318,9 @@ export const ENC_TYPE_BASE62 = 1; // new (tiny) encoding
  * @return - the encoded versionId
  */
 export function encode(str: string): string {
-    // default format without 'info' field will always be 27 characters
-    if (str.length === 27) {
+    // Legacy base62 version IDs (without 'info' field) are always 27 characters long.
+    // The new base62 format is 35 characters and includes the format marker at the end.
+    if (str.length === LEGACY_BASE62_DECODED_LENGTH || hasVersionIDFormat(str, versionIDFormat)) {
         return base62Encode(str);
     } // legacy format
     return hexEncode(str);
@@ -296,15 +336,19 @@ export function encode(str: string): string {
  */
 export function decode(str: string): string | Error {
     // default format is exactly 32 characters when encoded
-    if (str.length === 32) {
+    if (str.length === BASE62_ENCODED_LENGTH) {
         const decoded: string | Error = base62Decode(str);
-        if (typeof decoded === 'string' && decoded.length !== 27) {
-            return new Error(`decoded ${str} is not length 27`);
+        // Legacy base62 version IDs (without 'info' field) are always 27 characters long.
+        // The new base62 format is always 35 characters long.
+        if (typeof decoded === 'string' &&
+            ![LEGACY_BASE62_DECODED_LENGTH, BASE62_DECODED_LENGTH].includes(decoded.length)) {
+            return new Error(`decoded ${str} is not length ` +
+                `${LEGACY_BASE62_DECODED_LENGTH} or ${BASE62_DECODED_LENGTH}`);
         }
         return decoded;
     }
     // legacy format
-    if (str.length > 32) {
+    if (str.length > BASE62_ENCODED_LENGTH) {
         return hexDecode(str);
     }
     return new Error(`cannot decode str ${str.length}`);
diff --git a/lib/versioning/constants.ts b/lib/versioning/constants.ts
@@ -9,6 +9,7 @@ export enum BucketVersioningFormat {
 export const VersioningConstants = {
     VersionId: {
         Separator: '\0',
+        FormatMarker: '\x1E',
     },
     DbPrefixes: {
         Master: '\x7fM',
diff --git a/tests/unit/versioning/VersionID.spec.js b/tests/unit/versioning/VersionID.spec.js
@@ -22,29 +22,6 @@ 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.
@@ -55,7 +32,7 @@ describe('test generating versionIds', () => {
             assert.strictEqual(decoded.message, 'Non-base62 character');
         });
     });
-    describe('legaxy hex encoding', () => {
+    describe('legacy hex encoding', () => {
         VID.S3_VERSION_ID_ENCODING_TYPE = 'hex';
         const vids = generateRandomVIDs(count);
 
@@ -83,12 +60,39 @@ describe('test generating versionIds', () => {
             const encoded = vids.map(VID.encode);
             const decoded = encoded.map(VID.decode);
 
-            assert.strictEqual(vids.every(x => x.length > 27), true);
+            assert.strictEqual(vids.every(x => x.length > 35), true);
             assert.strictEqual(encoded.every(x => x.length > 32), true);
             assert.deepStrictEqual(vids, decoded);
         });
-    });
 
+        it('should not include format marker in legacy hex encoding', () => {
+            assert.strictEqual(vids.some(vid => vid.includes('\x1E')), false);
+        });
+
+        it('should encode and decode hex versionID with exactly Short ID length', () => {
+            const versionID = '98248620612400999999RG00001145.20.5'; // 35 characters long
+            const encoded = VID.encode(versionID);
+            assert.strictEqual(encoded.length > 32, true);
+            const decoded = VID.decode(encoded);
+            assert.strictEqual(decoded, versionID);
+        });
+
+        it('should encode and decode versionID with legacy Short ID length', () => {
+            const versionID = '98248620612400999999RG00001'; // 27 characters long
+            const encoded = VID.encode(versionID);
+            assert.strictEqual(encoded.length === 32, true);
+            const decoded = VID.decode(encoded);
+            assert.strictEqual(decoded, versionID);
+        });
+
+        it('should encode and decode Short ID', () => {
+            const versionID = '98248700112011999999RG00001enr984\x1E1'; // 35 characters long
+            const encoded = VID.encode(versionID);
+            assert.strictEqual(encoded.length === 32, true);
+            const decoded = VID.decode(encoded);
+            assert.strictEqual(decoded, versionID);
+        });
+    });
 
     describe('Short IDs', () => {
         VID.S3_VERSION_ID_ENCODING_TYPE = 'base62';
@@ -154,9 +158,25 @@ describe('test generating versionIds', () => {
         it('should encode and decode correctly with new 32 byte format', () => {
             const encoded = vids.map(vid => VID.encode(vid));
             const decoded = encoded.map(vid => VID.decode(vid));
-            assert(vids.every(x => x.length === 27));
+            assert(vids.every(x => x.length === 35));
             assert(encoded.every(x => x.length === 32));
             assert.deepStrictEqual(vids, decoded);
         });
+
+        it('should encode and decode legacy short versionID', () => {
+            const legacyVID = '98248620612400999999RG00001'; // 27 characters long
+            const encoded = VID.encode(legacyVID);
+            assert.strictEqual(encoded.length === 32, true);
+            const decoded = VID.decode(encoded);
+            assert.strictEqual(decoded, legacyVID);
+        });
+
+        it('should encode and decode legacy hex versionID', () => {
+            const legacyVID = '98248620612400999999RG00001someinformation';
+            const encoded = VID.encode(legacyVID);
+            assert.strictEqual(encoded.length > 32, true);
+            const decoded = VID.decode(encoded);
+            assert.strictEqual(decoded, legacyVID);
+        });
     });
 });
diff --git a/tests/unit/versioning/VersioningRequestProcessor.spec.js b/tests/unit/versioning/VersioningRequestProcessor.spec.js
