Skip to content

Commit fc70477

Browse files
committed
Use server-provided GC grace period for tombstone removal
`gcGracePeriod` is now set from connectionDetails received on CONNECTED event. Added to realtime in [1] Resolves PUB-1702 [1] https://ably.atlassian.net/browse/PUB-1127
1 parent 6366a52 commit fc70477

6 files changed

Lines changed: 69 additions & 10 deletions

File tree

src/plugins/objects/defaults.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
export const DEFAULTS = {
22
gcInterval: 1000 * 60 * 5, // 5 minutes
33
/**
4+
* The SDK will attempt to use the `gcGracePeriod` value provided by the server in the `connectionDetails` object of the `CONNECTED` event.
5+
* If the server does not provide this value, the SDK will fall back to this default value.
6+
*
47
* Must be > 2 minutes to ensure we keep tombstones long enough to avoid the possibility of receiving an operation
58
* with an earlier serial that would not have been applied if the tombstone still existed.
69
*

src/plugins/objects/livemap.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { dequal } from 'dequal';
22

33
import type { Bufferlike } from 'common/platform';
44
import type * as API from '../../../ably';
5-
import { DEFAULTS } from './defaults';
65
import { LiveObject, LiveObjectData, LiveObjectUpdate, LiveObjectUpdateNoop } from './liveobject';
76
import { ObjectId } from './objectid';
87
import {
@@ -531,7 +530,7 @@ export class LiveMap<T extends API.LiveMapType> extends LiveObject<LiveMapData,
531530

532531
const keysToDelete: string[] = [];
533532
for (const [key, value] of this._dataRef.data.entries()) {
534-
if (value.tombstone === true && Date.now() - value.tombstonedAt! >= DEFAULTS.gcGracePeriod) {
533+
if (value.tombstone === true && Date.now() - value.tombstonedAt! >= this._objects.gcGracePeriod) {
535534
keysToDelete.push(key);
536535
}
537536
}

src/plugins/objects/objects.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export interface OnObjectsEventResponse {
3737
export type BatchCallback = (batchContext: BatchContext) => void;
3838

3939
export class Objects {
40+
gcGracePeriod: number;
41+
4042
private _client: BaseClient;
4143
private _channel: RealtimeChannel;
4244
private _state: ObjectsState;
@@ -63,6 +65,11 @@ export class Objects {
6365
this._objectsPool = new ObjectsPool(this);
6466
this._syncObjectsDataPool = new SyncObjectsDataPool(this);
6567
this._bufferedObjectOperations = [];
68+
// use server-provided gcGracePeriod if available, and subscribe to new connectionDetails that can be emitted as part of the RTN24
69+
this.gcGracePeriod = this._channel.connectionManager.connectionDetails?.gcGracePeriod ?? DEFAULTS.gcGracePeriod;
70+
this._channel.connectionManager.on('connectiondetails', (details: Record<string, any>) => {
71+
this.gcGracePeriod = details.gcGracePeriod ?? DEFAULTS.gcGracePeriod;
72+
});
6673
}
6774

6875
/**

src/plugins/objects/objectspool.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export class ObjectsPool {
109109
// tombstoned objects should be removed from the pool if they have been tombstoned for longer than grace period.
110110
// by removing them from the local pool, Objects plugin no longer keeps a reference to those objects, allowing JS's
111111
// Garbage Collection to eventually free the memory for those objects, provided the user no longer references them either.
112-
if (obj.isTombstoned() && Date.now() - obj.tombstonedAt()! >= DEFAULTS.gcGracePeriod) {
112+
if (obj.isTombstoned() && Date.now() - obj.tombstonedAt()! >= this._objects.gcGracePeriod) {
113113
toDelete.push(objectId);
114114
continue;
115115
}

test/common/modules/private_api_recorder.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ define(['test/support/output_directory_paths'], function (outputDirectoryPaths)
7979
'read.Defaults.version',
8080
'read.LiveMap._dataRef.data',
8181
'read.EventEmitter.events',
82+
'read.Objects.gcGracePeriod',
8283
'read.Platform.Config.push',
8384
'read.ProtocolMessage.channelSerial',
8485
'read.Realtime._transports',
@@ -140,8 +141,8 @@ define(['test/support/output_directory_paths'], function (outputDirectoryPaths)
140141
'write.Defaults.ENDPOINT',
141142
'write.Defaults.ENVIRONMENT',
142143
'write.Defaults.wsConnectivityCheckUrl',
143-
'write.Objects._DEFAULTS.gcGracePeriod',
144144
'write.Objects._DEFAULTS.gcInterval',
145+
'write.Objects.gcGracePeriod',
145146
'write.Platform.Config.push', // This implies using a mock implementation of the internal IPlatformPushConfig interface. Our mock (in push_channel_transport.js) then interacts with internal objects and private APIs of public objects to implement this interface; I haven’t added annotations for that private API usage, since there wasn’t an easy way to pass test context information into the mock. I think that for now we can just say that if we wanted to get rid of this private API usage, then we’d need to remove this mock entirely.
146147
'write.auth.authOptions.requestHeaders',
147148
'write.auth.key',

test/realtime/objects.test.js

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ define(['ably', 'shared_helper', 'chai', 'objects', 'objects_helper'], function
1515
const objectsFixturesChannel = 'objects_fixtures';
1616
const nextTick = Ably.Realtime.Platform.Config.nextTick;
1717
const gcIntervalOriginal = ObjectsPlugin.Objects._DEFAULTS.gcInterval;
18-
const gcGracePeriodOriginal = ObjectsPlugin.Objects._DEFAULTS.gcGracePeriod;
1918

2019
function RealtimeWithObjects(helper, options) {
2120
return helper.AblyRealtime({ ...options, plugins: { Objects: ObjectsPlugin } });
@@ -4227,6 +4226,52 @@ define(['ably', 'shared_helper', 'chai', 'objects', 'objects_helper'], function
42274226
}, client);
42284227
});
42294228

4229+
it('gcGracePeriod is set from connectionDetails', async function () {
4230+
const helper = this.test.helper;
4231+
const client = RealtimeWithObjects(helper);
4232+
4233+
await helper.monitorConnectionThenCloseAndFinishAsync(async () => {
4234+
await client.connection.once('connected');
4235+
4236+
const channel = client.channels.get('channel', channelOptionsWithObjects());
4237+
const objects = channel.objects;
4238+
const connectionManager = client.connection.connectionManager;
4239+
const connectionDetails = connectionManager.connectionDetails;
4240+
4241+
// gcGracePeriod should be set after the initial connection
4242+
helper.recordPrivateApi('read.Objects.gcGracePeriod');
4243+
expect(objects.gcGracePeriod, 'Check gcGracePeriod is set after initial connection').to.exist;
4244+
helper.recordPrivateApi('read.Objects.gcGracePeriod');
4245+
expect(objects.gcGracePeriod).to.equal(
4246+
connectionDetails.gcGracePeriod,
4247+
'Check gcGracePeriod is set to equal connectionDetails.gcGracePeriod',
4248+
);
4249+
4250+
const connectionDetailsPromise = connectionManager.once('connectiondetails');
4251+
4252+
helper.recordPrivateApi('call.connectionManager.activeProtocol.getTransport');
4253+
helper.recordPrivateApi('call.transport.onProtocolMessage');
4254+
helper.recordPrivateApi('call.makeProtocolMessageFromDeserialized');
4255+
connectionManager.activeProtocol.getTransport().onProtocolMessage(
4256+
createPM({
4257+
action: 4, // CONNECTED
4258+
connectionDetails: {
4259+
...connectionDetails,
4260+
gcGracePeriod: 999,
4261+
},
4262+
}),
4263+
);
4264+
4265+
helper.recordPrivateApi('listen.connectionManager.connectiondetails');
4266+
await connectionDetailsPromise;
4267+
// wait for next tick to ensure the connectionDetails event was processed by Objects plugin
4268+
await new Promise((res) => nextTick(res));
4269+
4270+
helper.recordPrivateApi('read.Objects.gcGracePeriod');
4271+
expect(objects.gcGracePeriod).to.equal(999, 'Check gcGracePeriod is updated on new CONNECTED event');
4272+
}, client);
4273+
});
4274+
42304275
const tombstonesGCScenarios = [
42314276
// for the next tests we need to access the private API of Objects plugin in order to verify that tombstoned entities were indeed deleted after the GC grace period.
42324277
// public API hides that kind of information from the user and returns undefined for tombstoned entities even if realtime client still keeps a reference to them.
@@ -4333,8 +4378,6 @@ define(['ably', 'shared_helper', 'chai', 'objects', 'objects_helper'], function
43334378
try {
43344379
helper.recordPrivateApi('write.Objects._DEFAULTS.gcInterval');
43354380
ObjectsPlugin.Objects._DEFAULTS.gcInterval = 500;
4336-
helper.recordPrivateApi('write.Objects._DEFAULTS.gcGracePeriod');
4337-
ObjectsPlugin.Objects._DEFAULTS.gcGracePeriod = 250;
43384381

43394382
const objectsHelper = new ObjectsHelper(helper);
43404383
const client = RealtimeWithObjects(helper, clientOptions);
@@ -4346,6 +4389,11 @@ define(['ably', 'shared_helper', 'chai', 'objects', 'objects_helper'], function
43464389
await channel.attach();
43474390
const root = await objects.getRoot();
43484391

4392+
helper.recordPrivateApi('read.Objects.gcGracePeriod');
4393+
const gcGracePeriodOriginal = objects.gcGracePeriod;
4394+
helper.recordPrivateApi('write.Objects.gcGracePeriod');
4395+
objects.gcGracePeriod = 250;
4396+
43494397
// helper function to spy on the GC interval callback and wait for a specific number of GC cycles.
43504398
// returns a promise which will resolve when required number of cycles have happened.
43514399
const waitForGCCycles = (cycles) => {
@@ -4376,12 +4424,13 @@ define(['ably', 'shared_helper', 'chai', 'objects', 'objects_helper'], function
43764424
helper,
43774425
waitForGCCycles,
43784426
});
4427+
4428+
helper.recordPrivateApi('write.Objects.gcGracePeriod');
4429+
objects.gcGracePeriod = gcGracePeriodOriginal;
43794430
}, client);
43804431
} finally {
43814432
helper.recordPrivateApi('write.Objects._DEFAULTS.gcInterval');
43824433
ObjectsPlugin.Objects._DEFAULTS.gcInterval = gcIntervalOriginal;
4383-
helper.recordPrivateApi('write.Objects._DEFAULTS.gcGracePeriod');
4384-
ObjectsPlugin.Objects._DEFAULTS.gcGracePeriod = gcGracePeriodOriginal;
43854434
}
43864435
});
43874436

@@ -4622,7 +4671,7 @@ define(['ably', 'shared_helper', 'chai', 'objects', 'objects_helper'], function
46224671
*/
46234672
it('object message publish respects connectionDetails.maxMessageSize', async function () {
46244673
const helper = this.test.helper;
4625-
const client = RealtimeWithObjects(helper, { clientId: 'test' });
4674+
const client = RealtimeWithObjects(helper);
46264675

46274676
await helper.monitorConnectionThenCloseAndFinishAsync(async () => {
46284677
await client.connection.once('connected');

0 commit comments

Comments
 (0)