From a9b1df8bc3934324383175b81d5bdf475ddf2a62 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Fri, 15 Dec 2023 18:34:19 +0530 Subject: [PATCH 01/15] fix: do NOT add DELETE entries for keys and notifications to commitLog --- .../lib/src/keystore/hive_keystore.dart | 4 +++- .../lib/src/notification/at_notification_keystore.dart | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart b/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart index 875e8d212..a1d4caa31 100644 --- a/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart @@ -351,7 +351,9 @@ class HiveKeystore implements SecondaryKeyStore { for (String element in expiredKeys) { try { - await remove(element); + // delete entries for expired keys will not be added to the commitLog + // and will be handled on the client side + await remove(element, skipCommit: true); } on KeyNotFoundException { continue; } diff --git a/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart b/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart index 1c828c8cf..39fe22dc5 100644 --- a/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart @@ -115,7 +115,8 @@ class AtNotificationKeystore var expiredKeys = await getExpiredKeys(); if (expiredKeys.isNotEmpty) { await Future.forEach(expiredKeys, (expiredKey) async { - await remove(expiredKey); + // Delete entries will not be added to commitLog + await remove(expiredKey, skipCommit: true); }); } else { _logger.finest('notification key store. No expired notifications'); From dbd16cc1973498102c0041716e3208e0b330d84b Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Wed, 20 Dec 2023 15:59:15 +0530 Subject: [PATCH 02/15] test: add unit tests to verify changes --- .../test/hive_key_expiry_check.dart | 124 ++++++++++++++---- 1 file changed, 102 insertions(+), 22 deletions(-) diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index a0615a47c..ebcd88b66 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -1,38 +1,118 @@ import 'dart:io'; +import 'package:at_commons/at_commons.dart'; import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart'; +import 'package:test/expect.dart'; +import 'package:test/scaffolding.dart'; void main() async { + var storageDir = '${Directory.current.path}/test/hive'; + String atsign = '@test_user_1'; + var keyStore; + + group('test scenarios for expired keys', () { + setUp(() async { + var keyStoreManager = await getKeystoreManager(storageDir, atsign); + keyStore = keyStoreManager.getKeyStore(); + assert(keyStore != null); + }); + + test('fetch expired key returns throws exception', () async { + String key = '123$atsign'; + var atData = AtData()..data = 'abc'; + await keyStore?.put(key, atData, time_to_live: 10 * 1000); + var atDataResponse = await keyStore?.get(key); + print(atDataResponse?.data); + assert(atDataResponse?.data == 'abc'); + stdout.writeln('Sleeping for 1min10s'); + var expiredKeyData = await Future.delayed( + Duration(minutes: 1, seconds: 10), () => getKey(keyStore, key)); + expect( + () async => expiredKeyData, + throwsA(predicate((e) => + e.toString().contains('123$atsign does not exist in keystore')))); + print(expiredKeyData); + }, timeout: Timeout(Duration(minutes: 2))); + + test('ensure expired keys deletion entry is not added to commitLog', + () async { + String key = 'no_commit_log_test$atsign'; + var atData = AtData()..data = 'randomDataString'; + await keyStore?.put(key, atData, time_to_live: 2000); + // ensure key is inserted + expect((await keyStore.get(key)).data, atData.data); + await Future.delayed(Duration(seconds: 4)); + await keyStore?.deleteExpiredKeys(); + // ensure that the key is expired + expect( + () async => await keyStore.get(key), + throwsA(predicate((e) => e.toString().contains( + 'no_commit_log_test@test_user_1 does not exist in keystore')))); + + expect(keyStore?.commitLog.getSize(), 1); //commitLog has 1 entries; indicating that + // deletion of expired keys has NOT been added to the commitLog + }); + + test( + 'manually deleted keys add a commitEntry to commitLog', + () async { + // -----------------insert key 1 that expires in 100ms + String key1 = 'no_commit_1$atsign'; + var atData = AtData()..data = 'randomDataString1'; + int seqNum = await keyStore?.put(key1, atData, time_to_live: 100); + await Future.delayed(Duration(seconds: 1)); + await keyStore?.deleteExpiredKeys(); + // ensure that the key is expired + expect( + () async => await keyStore.get(key1), + throwsA(predicate((p0) => p0 is KeyNotFoundException))); + // ------------------insert key2 that is manually deleted + String key2 = 'no_commit_2$atsign'; + atData = AtData()..data = 'randomDataString2'; + seqNum = await keyStore?.put(key2, atData); + seqNum = await keyStore?.remove(key2); + // ensure that the key does not exist in keystore + expect( + () async => await keyStore.get(key2), + throwsA(predicate((p0) => p0 is KeyNotFoundException))); + + expect(keyStore?.commitLog.getSize(), 1); //commitLog has 1 entry; indicating that + // deletion of expired keys has NOT been added to the commitLog but the manual + // delete operation added a commit to the commitLog + }); + + tearDown(() async => await tearDownFunc()); + }); +} + +Future getKey(keyStore, key) async { + AtData? atData = await keyStore.get(key); + return atData?.data; +} + +Future getKeystoreManager(storageDir, atsign) async { var secondaryPersistenceStore = SecondaryPersistenceStoreFactory.getInstance() - .getSecondaryPersistenceStore('@test_user_1')!; + .getSecondaryPersistenceStore(atsign)!; var manager = secondaryPersistenceStore.getHivePersistenceManager()!; - await manager.init('test/hive'); + await manager.init(storageDir); manager.scheduleKeyExpireTask(1); var keyStoreManager = secondaryPersistenceStore.getSecondaryKeyStoreManager()!; var keyStore = secondaryPersistenceStore.getSecondaryKeyStore()!; - var commitLogKeyStore = CommitLogKeyStore('@test_user_1'); - await commitLogKeyStore.init('test/hive/commit'); - keyStore.commitLog = AtCommitLog(commitLogKeyStore); + var commitLog = await AtCommitLogManagerImpl.getInstance() + .getCommitLog(atsign, commitLogPath: storageDir, enableCommitId: true); + // await commitLogKeyStore.init(storageDir); + // await commitLogKeyStore.initialize(); + keyStore.commitLog = commitLog; keyStoreManager.keyStore = keyStore; - var atData = AtData(); - atData.data = 'abc'; - await keyStoreManager - .getKeyStore() - .put('123', atData, time_to_live: 30 * 1000); - print('end'); - var atDataResponse = await keyStoreManager.getKeyStore().get('123'); - print(atDataResponse?.data); - assert(atDataResponse?.data == 'abc'); - var expiredKey = - await Future.delayed(Duration(minutes: 2), () => getKey(keyStoreManager)); - assert(expiredKey == null); - print(expiredKey); - exit(0); + return keyStoreManager; } -Future getKey(keyStoreManager) async { - AtData? atData = await keyStoreManager.getKeyStore().get('123'); - return atData?.data; +Future tearDownFunc() async { + await AtCommitLogManagerImpl.getInstance().close(); + var isExists = await Directory('test/hive/').exists(); + if (isExists) { + Directory('test/hive').deleteSync(recursive: true); + } } From e501dd205c8367fd2b6e03bf8626e280e0b1708a Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 21 Dec 2023 03:36:36 +0530 Subject: [PATCH 03/15] test: fix tests --- .../at_notification_keystore.dart | 2 +- .../test/hive_key_expiry_check.dart | 42 +++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart b/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart index 39fe22dc5..bfcd2ef46 100644 --- a/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/notification/at_notification_keystore.dart @@ -115,7 +115,7 @@ class AtNotificationKeystore var expiredKeys = await getExpiredKeys(); if (expiredKeys.isNotEmpty) { await Future.forEach(expiredKeys, (expiredKey) async { - // Delete entries will not be added to commitLog + // Delete entries for expired keys will not be added to commitLog await remove(expiredKey, skipCommit: true); }); } else { diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index ebcd88b66..fa39a8040 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -2,18 +2,19 @@ import 'dart:io'; import 'package:at_commons/at_commons.dart'; import 'package:at_persistence_secondary_server/at_persistence_secondary_server.dart'; +import 'package:at_persistence_secondary_server/src/keystore/hive_keystore.dart'; import 'package:test/expect.dart'; import 'package:test/scaffolding.dart'; void main() async { var storageDir = '${Directory.current.path}/test/hive'; String atsign = '@test_user_1'; - var keyStore; + HiveKeystore? keyStore; group('test scenarios for expired keys', () { setUp(() async { var keyStoreManager = await getKeystoreManager(storageDir, atsign); - keyStore = keyStoreManager.getKeyStore(); + keyStore = keyStoreManager.getKeyStore() as HiveKeystore?; assert(keyStore != null); }); @@ -25,13 +26,11 @@ void main() async { print(atDataResponse?.data); assert(atDataResponse?.data == 'abc'); stdout.writeln('Sleeping for 1min10s'); - var expiredKeyData = await Future.delayed( - Duration(minutes: 1, seconds: 10), () => getKey(keyStore, key)); + await Future.delayed(Duration(minutes: 1, seconds: 10)); expect( - () async => expiredKeyData, + () async => getKey(keyStore, key), throwsA(predicate((e) => e.toString().contains('123$atsign does not exist in keystore')))); - print(expiredKeyData); }, timeout: Timeout(Duration(minutes: 2))); test('ensure expired keys deletion entry is not added to commitLog', @@ -40,16 +39,17 @@ void main() async { var atData = AtData()..data = 'randomDataString'; await keyStore?.put(key, atData, time_to_live: 2000); // ensure key is inserted - expect((await keyStore.get(key)).data, atData.data); + expect((await keyStore?.get(key))?.data, atData.data); + await Future.delayed(Duration(seconds: 4)); await keyStore?.deleteExpiredKeys(); // ensure that the key is expired expect( - () async => await keyStore.get(key), + () async => await keyStore?.get(key), throwsA(predicate((e) => e.toString().contains( 'no_commit_log_test@test_user_1 does not exist in keystore')))); - expect(keyStore?.commitLog.getSize(), 1); //commitLog has 1 entries; indicating that + expect(keyStore?.commitLog?.entriesCount(), 1); //commitLog has 1 entries; indicating that // deletion of expired keys has NOT been added to the commitLog }); @@ -59,24 +59,27 @@ void main() async { // -----------------insert key 1 that expires in 100ms String key1 = 'no_commit_1$atsign'; var atData = AtData()..data = 'randomDataString1'; - int seqNum = await keyStore?.put(key1, atData, time_to_live: 100); + int? seqNum = await keyStore?.put(key1, atData, time_to_live: 100); + print(seqNum); await Future.delayed(Duration(seconds: 1)); await keyStore?.deleteExpiredKeys(); // ensure that the key is expired expect( - () async => await keyStore.get(key1), + () async => await keyStore?.get(key1), throwsA(predicate((p0) => p0 is KeyNotFoundException))); // ------------------insert key2 that is manually deleted String key2 = 'no_commit_2$atsign'; atData = AtData()..data = 'randomDataString2'; seqNum = await keyStore?.put(key2, atData); + print(seqNum); seqNum = await keyStore?.remove(key2); - // ensure that the key does not exist in keystore + // ensure that the second key does not exist in keystore expect( - () async => await keyStore.get(key2), - throwsA(predicate((p0) => p0 is KeyNotFoundException))); + () async => await keyStore?.get(key2), + throwsA(predicate((e) => e is KeyNotFoundException))); - expect(keyStore?.commitLog.getSize(), 1); //commitLog has 1 entry; indicating that + listCommitEntries((await AtCommitLogManagerImpl.getInstance().getCommitLog(atsign,commitLogPath: storageDir, enableCommitId: true))!.getEntries(0)); + expect(keyStore?.commitLog?.entriesCount(), 2); //commitLog has 2 entry; indicating that // deletion of expired keys has NOT been added to the commitLog but the manual // delete operation added a commit to the commitLog }); @@ -96,19 +99,22 @@ Future getKeystoreManager(storageDir, atsign) async { var manager = secondaryPersistenceStore.getHivePersistenceManager()!; await manager.init(storageDir); manager.scheduleKeyExpireTask(1); - var keyStoreManager = secondaryPersistenceStore.getSecondaryKeyStoreManager()!; var keyStore = secondaryPersistenceStore.getSecondaryKeyStore()!; var commitLog = await AtCommitLogManagerImpl.getInstance() .getCommitLog(atsign, commitLogPath: storageDir, enableCommitId: true); - // await commitLogKeyStore.init(storageDir); - // await commitLogKeyStore.initialize(); keyStore.commitLog = commitLog; keyStoreManager.keyStore = keyStore; return keyStoreManager; } +void listCommitEntries(Iterator> commitLogIterator){ + while(commitLogIterator.moveNext()) { + print(commitLogIterator.current); + } +} + Future tearDownFunc() async { await AtCommitLogManagerImpl.getInstance().close(); var isExists = await Directory('test/hive/').exists(); From 2e4099c5bdf329569351e9b7310165b82223165b Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 21 Dec 2023 18:17:02 +0530 Subject: [PATCH 04/15] build: update changelog and pub version to 3.0.60 --- packages/at_persistence_secondary_server/CHANGELOG.md | 2 ++ packages/at_persistence_secondary_server/pubspec.yaml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/at_persistence_secondary_server/CHANGELOG.md b/packages/at_persistence_secondary_server/CHANGELOG.md index 2c9e79092..529599f9a 100644 --- a/packages/at_persistence_secondary_server/CHANGELOG.md +++ b/packages/at_persistence_secondary_server/CHANGELOG.md @@ -1,3 +1,5 @@ +## 3.0.60 +_ feat: delete entries for expired keys are not committed to the commitLog ## 3.0.59 - fix: When checking namespace authorization, gracefully handle any malformed keys which happen to be in the commit log for historical reasons diff --git a/packages/at_persistence_secondary_server/pubspec.yaml b/packages/at_persistence_secondary_server/pubspec.yaml index 2c438a24f..80dc18978 100644 --- a/packages/at_persistence_secondary_server/pubspec.yaml +++ b/packages/at_persistence_secondary_server/pubspec.yaml @@ -1,6 +1,6 @@ name: at_persistence_secondary_server description: A Dart library with the implementation classes for the persistence layer of the secondary server. -version: 3.0.59 +version: 3.0.60 repository: https://github.com/atsign-foundation/at_server homepage: https://docs.atsign.com/ From c14326ea9c3ee82b236a0ce2496b7be71e99481f Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Thu, 21 Dec 2023 18:18:55 +0530 Subject: [PATCH 05/15] fix: remove unnecessary code --- .../test/hive_key_expiry_check.dart | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index fa39a8040..45ca729a9 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -78,7 +78,6 @@ void main() async { () async => await keyStore?.get(key2), throwsA(predicate((e) => e is KeyNotFoundException))); - listCommitEntries((await AtCommitLogManagerImpl.getInstance().getCommitLog(atsign,commitLogPath: storageDir, enableCommitId: true))!.getEntries(0)); expect(keyStore?.commitLog?.entriesCount(), 2); //commitLog has 2 entry; indicating that // deletion of expired keys has NOT been added to the commitLog but the manual // delete operation added a commit to the commitLog @@ -109,12 +108,6 @@ Future getKeystoreManager(storageDir, atsign) async { return keyStoreManager; } -void listCommitEntries(Iterator> commitLogIterator){ - while(commitLogIterator.moveNext()) { - print(commitLogIterator.current); - } -} - Future tearDownFunc() async { await AtCommitLogManagerImpl.getInstance().close(); var isExists = await Directory('test/hive/').exists(); From 8cf2cceb1b9c68be3fbfc15d09e520a42383bcfb Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Fri, 29 Dec 2023 16:04:10 +0530 Subject: [PATCH 06/15] test: reduce expired check test delay to 23s --- .../test/hive_key_expiry_check.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index 45ca729a9..b1e2452b1 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -21,12 +21,12 @@ void main() async { test('fetch expired key returns throws exception', () async { String key = '123$atsign'; var atData = AtData()..data = 'abc'; - await keyStore?.put(key, atData, time_to_live: 10 * 1000); + await keyStore?.put(key, atData, time_to_live: 5 * 1000); var atDataResponse = await keyStore?.get(key); print(atDataResponse?.data); assert(atDataResponse?.data == 'abc'); - stdout.writeln('Sleeping for 1min10s'); - await Future.delayed(Duration(minutes: 1, seconds: 10)); + stdout.writeln('Sleeping for 23s'); + await Future.delayed(Duration(seconds: 23)); expect( () async => getKey(keyStore, key), throwsA(predicate((e) => @@ -97,7 +97,7 @@ Future getKeystoreManager(storageDir, atsign) async { .getSecondaryPersistenceStore(atsign)!; var manager = secondaryPersistenceStore.getHivePersistenceManager()!; await manager.init(storageDir); - manager.scheduleKeyExpireTask(1); + manager.scheduleKeyExpireTask(null, runTimeInterval: Duration(seconds: 10)); var keyStoreManager = secondaryPersistenceStore.getSecondaryKeyStoreManager()!; var keyStore = secondaryPersistenceStore.getSecondaryKeyStore()!; From c29b1377fc65b44c578368f419673cfa46ac8755 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Fri, 29 Dec 2023 16:05:23 +0530 Subject: [PATCH 07/15] feat: optionally allow Duration as method param for scheduleKeyExpireTask() --- .../lib/src/keystore/hive_manager.dart | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart index be7e6f82b..cd6c55440 100644 --- a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart +++ b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart @@ -113,9 +113,15 @@ class HivePersistenceManager with HiveBase { } //TODO change into to Duration and construct cron string dynamically - void scheduleKeyExpireTask(int runFrequencyMins) { + void scheduleKeyExpireTask(int? runFrequencyMins, {Duration? runTimeInterval}) { logger.finest('scheduleKeyExpireTask starting cron job.'); - _cron.schedule(Schedule.parse('*/$runFrequencyMins * * * *'), () async { + Schedule schedule; + if(runTimeInterval != null){ + schedule = Schedule(seconds: runTimeInterval.inSeconds); + } else { + schedule = Schedule.parse('*/$runFrequencyMins * * * *'); + } + _cron.schedule(schedule, () async { await Future.delayed(Duration(seconds: _random.nextInt(12))); var hiveKeyStore = SecondaryPersistenceStoreFactory.getInstance() .getSecondaryPersistenceStore(_atsign)! From 06b3ac80d4437d029eae96fee5181e3fb1c15045 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Fri, 5 Jan 2024 00:52:45 +0530 Subject: [PATCH 08/15] feat: introduce flag to optimize commits for expired keys --- .../lib/src/keystore/hive_keystore.dart | 7 ++++--- .../lib/src/keystore/hive_manager.dart | 5 +++-- .../test/hive_key_expiry_check.dart | 9 ++++----- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart b/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart index a1d4caa31..996e23053 100644 --- a/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart @@ -41,7 +41,6 @@ class HiveKeystore implements SecondaryKeyStore { } @Deprecated("Use [initialize]") - /// Deprecated. Use [initialize] Future init() async { await initialize(); @@ -341,7 +340,9 @@ class HiveKeystore implements SecondaryKeyStore { @override @server - Future deleteExpiredKeys() async { + @client + Future deleteExpiredKeys({bool? optimizeCommits = false}) async { + logger.finer('Removing expired keys'); bool result = true; try { List expiredKeys = await getExpiredKeys(); @@ -353,7 +354,7 @@ class HiveKeystore implements SecondaryKeyStore { try { // delete entries for expired keys will not be added to the commitLog // and will be handled on the client side - await remove(element, skipCommit: true); + await remove(element, skipCommit: optimizeCommits!); } on KeyNotFoundException { continue; } diff --git a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart index cd6c55440..7194f959f 100644 --- a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart +++ b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart @@ -113,7 +113,7 @@ class HivePersistenceManager with HiveBase { } //TODO change into to Duration and construct cron string dynamically - void scheduleKeyExpireTask(int? runFrequencyMins, {Duration? runTimeInterval}) { + void scheduleKeyExpireTask(int? runFrequencyMins, {Duration? runTimeInterval, bool optimizeCommits = false}) { logger.finest('scheduleKeyExpireTask starting cron job.'); Schedule schedule; if(runTimeInterval != null){ @@ -121,12 +121,13 @@ class HivePersistenceManager with HiveBase { } else { schedule = Schedule.parse('*/$runFrequencyMins * * * *'); } + schedule = Schedule(seconds: runTimeInterval!.inSeconds); _cron.schedule(schedule, () async { await Future.delayed(Duration(seconds: _random.nextInt(12))); var hiveKeyStore = SecondaryPersistenceStoreFactory.getInstance() .getSecondaryPersistenceStore(_atsign)! .getSecondaryKeyStore()!; - await hiveKeyStore.deleteExpiredKeys(); + await hiveKeyStore.deleteExpiredKeys(optimizeCommits: optimizeCommits); }); } diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index b1e2452b1..4f554d783 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -23,7 +23,6 @@ void main() async { var atData = AtData()..data = 'abc'; await keyStore?.put(key, atData, time_to_live: 5 * 1000); var atDataResponse = await keyStore?.get(key); - print(atDataResponse?.data); assert(atDataResponse?.data == 'abc'); stdout.writeln('Sleeping for 23s'); await Future.delayed(Duration(seconds: 23)); @@ -31,7 +30,7 @@ void main() async { () async => getKey(keyStore, key), throwsA(predicate((e) => e.toString().contains('123$atsign does not exist in keystore')))); - }, timeout: Timeout(Duration(minutes: 2))); + }, timeout: Timeout(Duration(minutes: 1))); test('ensure expired keys deletion entry is not added to commitLog', () async { @@ -77,9 +76,9 @@ void main() async { expect( () async => await keyStore?.get(key2), throwsA(predicate((e) => e is KeyNotFoundException))); - + /// ToDo: need to verify specific comments rather than the entreies count expect(keyStore?.commitLog?.entriesCount(), 2); //commitLog has 2 entry; indicating that - // deletion of expired keys has NOT been added to the commitLog but the manual + // deletion of expired keys has NOT been added to the comList commits = keyStore?.commitLog?.mitLog but the manual // delete operation added a commit to the commitLog }); @@ -97,7 +96,7 @@ Future getKeystoreManager(storageDir, atsign) async { .getSecondaryPersistenceStore(atsign)!; var manager = secondaryPersistenceStore.getHivePersistenceManager()!; await manager.init(storageDir); - manager.scheduleKeyExpireTask(null, runTimeInterval: Duration(seconds: 10)); + manager.scheduleKeyExpireTask(null, runTimeInterval: Duration(seconds: 10), optimizeCommits: true); var keyStoreManager = secondaryPersistenceStore.getSecondaryKeyStoreManager()!; var keyStore = secondaryPersistenceStore.getSecondaryKeyStore()!; From b1c949376913a6f090efed26cf6099ce7bde78bd Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Fri, 5 Jan 2024 00:55:35 +0530 Subject: [PATCH 09/15] docs: update changelog --- packages/at_persistence_secondary_server/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/at_persistence_secondary_server/CHANGELOG.md b/packages/at_persistence_secondary_server/CHANGELOG.md index 8e7c0026a..c02bb5ff7 100644 --- a/packages/at_persistence_secondary_server/CHANGELOG.md +++ b/packages/at_persistence_secondary_server/CHANGELOG.md @@ -1,5 +1,6 @@ -## 3.0.60 +## 3.0.61 - feat: delete entries for expired keys are not committed to the commitLog [feature not enabled yet] +## 3.0.60 - build[deps]: Upgraded the following packages: - at_commons to v4.0.0 - at_utils to v3.0.16 From 1f0f7e3d7c291aaa1d1a7956bec5d81ac5b64049 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Fri, 5 Jan 2024 02:21:06 +0530 Subject: [PATCH 10/15] tests: more tests --- .../test/hive_key_expiry_check.dart | 105 ++++++++++++++---- 1 file changed, 85 insertions(+), 20 deletions(-) diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index 4f554d783..cfd3611c5 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -11,9 +11,11 @@ void main() async { String atsign = '@test_user_1'; HiveKeystore? keyStore; - group('test scenarios for expired keys', () { + group('test scenarios for expired keys - CASE: optimizeCommits set to TRUE', + () { + // verifies that deletion of expired keys does NOT create/update commit entries setUp(() async { - var keyStoreManager = await getKeystoreManager(storageDir, atsign); + var keyStoreManager = await getKeystoreManager(storageDir, atsign, optimizeCommits: true); keyStore = keyStoreManager.getKeyStore() as HiveKeystore?; assert(keyStore != null); }); @@ -37,35 +39,37 @@ void main() async { String key = 'no_commit_log_test$atsign'; var atData = AtData()..data = 'randomDataString'; await keyStore?.put(key, atData, time_to_live: 2000); - // ensure key is inserted expect((await keyStore?.get(key))?.data, atData.data); await Future.delayed(Duration(seconds: 4)); - await keyStore?.deleteExpiredKeys(); + await keyStore?.deleteExpiredKeys(optimizeCommits: true); // ensure that the key is expired expect( () async => await keyStore?.get(key), throwsA(predicate((e) => e.toString().contains( 'no_commit_log_test@test_user_1 does not exist in keystore')))); - - expect(keyStore?.commitLog?.entriesCount(), 1); //commitLog has 1 entries; indicating that - // deletion of expired keys has NOT been added to the commitLog + AtCommitLog? commitLog = keyStore?.commitLog as AtCommitLog; + expect( + commitLog.getLatestCommitEntry(key)?.operation, CommitOp.UPDATE_ALL); + // the latest commit entry is one with an UPDATE_ALL op which indicates that + // the deleteExpiredKeys did not add a DELETE commitEntry to the commitLog + expect(commitLog.entriesCount(), 1); }); - test( - 'manually deleted keys add a commitEntry to commitLog', - () async { + test('manually deleted keys add a commitEntry to commitLog', () async { + AtCommitLog? commitLog = keyStore?.commitLog as AtCommitLog; // -----------------insert key 1 that expires in 100ms String key1 = 'no_commit_1$atsign'; var atData = AtData()..data = 'randomDataString1'; int? seqNum = await keyStore?.put(key1, atData, time_to_live: 100); print(seqNum); await Future.delayed(Duration(seconds: 1)); - await keyStore?.deleteExpiredKeys(); + await keyStore?.deleteExpiredKeys(optimizeCommits: true); // ensure that the key is expired - expect( - () async => await keyStore?.get(key1), + expect(() async => await keyStore?.get(key1), throwsA(predicate((p0) => p0 is KeyNotFoundException))); + expect( + commitLog.getLatestCommitEntry(key1)?.operation, CommitOp.UPDATE_ALL); // ------------------insert key2 that is manually deleted String key2 = 'no_commit_2$atsign'; atData = AtData()..data = 'randomDataString2'; @@ -73,13 +77,72 @@ void main() async { print(seqNum); seqNum = await keyStore?.remove(key2); // ensure that the second key does not exist in keystore + expect(() async => await keyStore?.get(key2), + throwsA(predicate((e) => e is KeyNotFoundException))); + expect(commitLog.getLatestCommitEntry(key2)?.operation, CommitOp.DELETE); + // the latest commitEntry for key2 has CommitOp.DELETE indicating that the commits are not being + // skipped for the keys that are not deleted as part of deleteExpiredKeys() + expect(keyStore?.commitLog?.entriesCount(), 2); + }); + + tearDown(() async => await tearDownFunc()); + }); + + group('test scenarios for expired keys - CASE: optimizeCommits set to FALSE', + () { + // verifies that deletion of expired keys creates/updates commit entries + setUp(() async { + var keyStoreManager = await getKeystoreManager(storageDir, atsign, optimizeCommits: false); + keyStore = keyStoreManager.getKeyStore() as HiveKeystore?; + assert(keyStore != null); + }); + + test('ensure expired keys deletion entry is not added to commitLog', + () async { + AtCommitLog? commitLog = keyStore?.commitLog as AtCommitLog; + String key = 'commit_test$atsign'; + var atData = AtData()..data = 'randomDataString'; + await keyStore?.put(key, atData, time_to_live: 2000); + // ensure key is inserted + expect((await keyStore?.get(key))?.data, atData.data); + + await Future.delayed(Duration(seconds: 4)); + await keyStore?.deleteExpiredKeys(); + // ensure that the key is expired expect( - () async => await keyStore?.get(key2), + () async => await keyStore?.get(key), + throwsA(predicate((e) => + e.toString().contains('$key does not exist in keystore')))); + + expect(commitLog.getLatestCommitEntry(key)?.operation, CommitOp.DELETE); + expect(commitLog.entriesCount(), 1); + }); + + test('manually deleted keys add a commitEntry to commitLog', () async { + AtCommitLog? commitLog = keyStore?.commitLog as AtCommitLog; + // -----------------insert key 1 that expires in 100ms + String key1 = 'no_commit_3$atsign'; + var atData = AtData()..data = 'randomDataString1'; + int? seqNum = await keyStore?.put(key1, atData, time_to_live: 100); + print(seqNum); + await Future.delayed(Duration(seconds: 1)); + await keyStore?.deleteExpiredKeys(); + // ensure that the key is expired + expect(() async => await keyStore?.get(key1), + throwsA(predicate((p0) => p0 is KeyNotFoundException))); + expect(commitLog.getLatestCommitEntry(key1)?.operation, CommitOp.DELETE); + // ------------------insert key2 that is manually deleted + String key2 = 'no_commit_4$atsign'; + atData = AtData()..data = 'randomDataString2'; + seqNum = await keyStore?.put(key2, atData); + print(seqNum); + seqNum = await keyStore?.remove(key2); + // ensure that the second key does not exist in keystore + expect(() async => await keyStore?.get(key2), throwsA(predicate((e) => e is KeyNotFoundException))); - /// ToDo: need to verify specific comments rather than the entreies count - expect(keyStore?.commitLog?.entriesCount(), 2); //commitLog has 2 entry; indicating that - // deletion of expired keys has NOT been added to the comList commits = keyStore?.commitLog?.mitLog but the manual - // delete operation added a commit to the commitLog + expect(commitLog.getLatestCommitEntry(key2)?.operation, CommitOp.DELETE); + expect(keyStore?.commitLog?.entriesCount(), + 2); }); tearDown(() async => await tearDownFunc()); @@ -91,12 +154,14 @@ Future getKey(keyStore, key) async { return atData?.data; } -Future getKeystoreManager(storageDir, atsign) async { +Future getKeystoreManager( + storageDir, atsign, {required bool optimizeCommits}) async { var secondaryPersistenceStore = SecondaryPersistenceStoreFactory.getInstance() .getSecondaryPersistenceStore(atsign)!; var manager = secondaryPersistenceStore.getHivePersistenceManager()!; await manager.init(storageDir); - manager.scheduleKeyExpireTask(null, runTimeInterval: Duration(seconds: 10), optimizeCommits: true); + manager.scheduleKeyExpireTask(null, + runTimeInterval: Duration(seconds: 10), optimizeCommits: optimizeCommits); var keyStoreManager = secondaryPersistenceStore.getSecondaryKeyStoreManager()!; var keyStore = secondaryPersistenceStore.getSecondaryKeyStore()!; From 43596d88eb0a7f2c1f79fb4efd4105517ea2318c Mon Sep 17 00:00:00 2001 From: Sri Teja T <50960424+srieteja@users.noreply.github.com> Date: Tue, 9 Jan 2024 18:59:40 +0530 Subject: [PATCH 11/15] hive_manager: remove incorrect schedule snippet --- .../lib/src/keystore/hive_manager.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart index 7194f959f..ca7aae237 100644 --- a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart +++ b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart @@ -121,7 +121,6 @@ class HivePersistenceManager with HiveBase { } else { schedule = Schedule.parse('*/$runFrequencyMins * * * *'); } - schedule = Schedule(seconds: runTimeInterval!.inSeconds); _cron.schedule(schedule, () async { await Future.delayed(Duration(seconds: _random.nextInt(12))); var hiveKeyStore = SecondaryPersistenceStoreFactory.getInstance() From 551b2494c9c6c1d19d83e6b8566c8cfebac8d827 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Fri, 19 Jan 2024 16:35:23 +0530 Subject: [PATCH 12/15] reformat: run dart formatter --- .../test/hive_key_expiry_check.dart | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index cfd3611c5..875d7f5c1 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -15,7 +15,8 @@ void main() async { () { // verifies that deletion of expired keys does NOT create/update commit entries setUp(() async { - var keyStoreManager = await getKeystoreManager(storageDir, atsign, optimizeCommits: true); + var keyStoreManager = + await getKeystoreManager(storageDir, atsign, optimizeCommits: true); keyStore = keyStoreManager.getKeyStore() as HiveKeystore?; assert(keyStore != null); }); @@ -90,9 +91,10 @@ void main() async { group('test scenarios for expired keys - CASE: optimizeCommits set to FALSE', () { - // verifies that deletion of expired keys creates/updates commit entries - setUp(() async { - var keyStoreManager = await getKeystoreManager(storageDir, atsign, optimizeCommits: false); + // verifies that deletion of expired keys creates/updates commit entries + setUp(() async { + var keyStoreManager = + await getKeystoreManager(storageDir, atsign, optimizeCommits: false); keyStore = keyStoreManager.getKeyStore() as HiveKeystore?; assert(keyStore != null); }); @@ -141,8 +143,7 @@ void main() async { expect(() async => await keyStore?.get(key2), throwsA(predicate((e) => e is KeyNotFoundException))); expect(commitLog.getLatestCommitEntry(key2)?.operation, CommitOp.DELETE); - expect(keyStore?.commitLog?.entriesCount(), - 2); + expect(keyStore?.commitLog?.entriesCount(), 2); }); tearDown(() async => await tearDownFunc()); @@ -154,8 +155,8 @@ Future getKey(keyStore, key) async { return atData?.data; } -Future getKeystoreManager( - storageDir, atsign, {required bool optimizeCommits}) async { +Future getKeystoreManager(storageDir, atsign, + {required bool optimizeCommits}) async { var secondaryPersistenceStore = SecondaryPersistenceStoreFactory.getInstance() .getSecondaryPersistenceStore(atsign)!; var manager = secondaryPersistenceStore.getHivePersistenceManager()!; From 6289b0d5f6fb65f177e0c855594708d1a3652874 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Mon, 19 Feb 2024 20:04:51 +0530 Subject: [PATCH 13/15] build: update version in pubspec to 3.0.61 --- packages/at_persistence_secondary_server/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/at_persistence_secondary_server/pubspec.yaml b/packages/at_persistence_secondary_server/pubspec.yaml index ce94fd209..fae0eb773 100644 --- a/packages/at_persistence_secondary_server/pubspec.yaml +++ b/packages/at_persistence_secondary_server/pubspec.yaml @@ -1,6 +1,6 @@ name: at_persistence_secondary_server description: A Dart library with the implementation classes for the persistence layer of the secondary server. -version: 3.0.60 +version: 3.0.61 repository: https://github.com/atsign-foundation/at_server homepage: https://docs.atsign.com/ From fefa7d41067053944799aff620786d1a84b19694 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Tue, 20 Feb 2024 20:42:57 +0530 Subject: [PATCH 14/15] refactor: rename optimizeCommits -> skipCommits --- .../lib/src/keystore/hive_keystore.dart | 6 +++--- .../lib/src/keystore/hive_manager.dart | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart b/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart index 996e23053..ed695982a 100644 --- a/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart +++ b/packages/at_persistence_secondary_server/lib/src/keystore/hive_keystore.dart @@ -341,7 +341,7 @@ class HiveKeystore implements SecondaryKeyStore { @override @server @client - Future deleteExpiredKeys({bool? optimizeCommits = false}) async { + Future deleteExpiredKeys({bool? skipCommit = false}) async { logger.finer('Removing expired keys'); bool result = true; try { @@ -353,8 +353,8 @@ class HiveKeystore implements SecondaryKeyStore { for (String element in expiredKeys) { try { // delete entries for expired keys will not be added to the commitLog - // and will be handled on the client side - await remove(element, skipCommit: optimizeCommits!); + // Removal of expired keys will be handled on the client side + await remove(element, skipCommit: skipCommit!); } on KeyNotFoundException { continue; } diff --git a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart index ca7aae237..ca5391338 100644 --- a/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart +++ b/packages/at_persistence_secondary_server/lib/src/keystore/hive_manager.dart @@ -113,7 +113,7 @@ class HivePersistenceManager with HiveBase { } //TODO change into to Duration and construct cron string dynamically - void scheduleKeyExpireTask(int? runFrequencyMins, {Duration? runTimeInterval, bool optimizeCommits = false}) { + void scheduleKeyExpireTask(int? runFrequencyMins, {Duration? runTimeInterval, bool skipCommits = false}) { logger.finest('scheduleKeyExpireTask starting cron job.'); Schedule schedule; if(runTimeInterval != null){ @@ -126,7 +126,7 @@ class HivePersistenceManager with HiveBase { var hiveKeyStore = SecondaryPersistenceStoreFactory.getInstance() .getSecondaryPersistenceStore(_atsign)! .getSecondaryKeyStore()!; - await hiveKeyStore.deleteExpiredKeys(optimizeCommits: optimizeCommits); + await hiveKeyStore.deleteExpiredKeys(skipCommit: skipCommits); }); } From 7da4a371fcee2cd9fe10f13c8d6cbbd908584d18 Mon Sep 17 00:00:00 2001 From: Srie Teja Date: Tue, 20 Feb 2024 20:53:21 +0530 Subject: [PATCH 15/15] tests: rename optimizeCommits -> skipCommits --- .../test/hive_key_expiry_check.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart index 875d7f5c1..e39cf0f2f 100644 --- a/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart +++ b/packages/at_persistence_secondary_server/test/hive_key_expiry_check.dart @@ -43,7 +43,7 @@ void main() async { expect((await keyStore?.get(key))?.data, atData.data); await Future.delayed(Duration(seconds: 4)); - await keyStore?.deleteExpiredKeys(optimizeCommits: true); + await keyStore?.deleteExpiredKeys(skipCommit: true); // ensure that the key is expired expect( () async => await keyStore?.get(key), @@ -65,7 +65,7 @@ void main() async { int? seqNum = await keyStore?.put(key1, atData, time_to_live: 100); print(seqNum); await Future.delayed(Duration(seconds: 1)); - await keyStore?.deleteExpiredKeys(optimizeCommits: true); + await keyStore?.deleteExpiredKeys(skipCommit: true); // ensure that the key is expired expect(() async => await keyStore?.get(key1), throwsA(predicate((p0) => p0 is KeyNotFoundException))); @@ -162,7 +162,7 @@ Future getKeystoreManager(storageDir, atsign, var manager = secondaryPersistenceStore.getHivePersistenceManager()!; await manager.init(storageDir); manager.scheduleKeyExpireTask(null, - runTimeInterval: Duration(seconds: 10), optimizeCommits: optimizeCommits); + runTimeInterval: Duration(seconds: 10), skipCommits: optimizeCommits); var keyStoreManager = secondaryPersistenceStore.getSecondaryKeyStoreManager()!; var keyStore = secondaryPersistenceStore.getSecondaryKeyStore()!;