alex-plekhanov commented on a change in pull request #7941:
URL: https://github.com/apache/ignite/pull/7941#discussion_r494384542
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/encryption/GridEncryptionManager.java
##########
@@ -821,11 +1181,57 @@ private void writeKeysToMetaStore(boolean writeAll)
throws IgniteCheckedExceptio
if (writeAll)
metaStorage.write(MASTER_KEY_NAME_PREFIX,
getSpi().getMasterKeyName());
- for (Map.Entry<Integer, Serializable> entry : grpEncKeys.entrySet()) {
- if (!writeAll && metaStorage.read(ENCRYPTION_KEY_PREFIX +
entry.getKey()) != null)
+ if (!reencryptGroupsForced.isEmpty())
+ writeTrackedWalIdxsToMetaStore();
+
+ for (Integer grpId : grpKeys.groupIds()) {
+ if (!writeAll && !reencryptGroupsForced.containsKey(grpId) &&
+ metaStorage.read(ENCRYPTION_KEYS_PREFIX + grpId) != null)
continue;
- writeToMetaStore(entry.getKey(),
getSpi().encryptKey(entry.getValue()));
+ writeGroupKeysToMetaStore(grpId);
+ }
+ }
+
+ /**
+ * Writes cache group encryption keys to metastore.
+ *
+ * @param grpId Cache group ID.
+ */
+ private void writeGroupKeysToMetaStore(int grpId) throws
IgniteCheckedException {
+ assert Thread.holdsLock(metaStorageMux);
+
+ if (metaStorage == null || !writeToMetaStoreEnabled || stopped)
+ return;
+
+ List<GroupKeyEncrypted> keysEncrypted = withMasterKeyChangeReadLock(()
-> grpKeys.getAll(grpId));
Review comment:
Different locks order, deadlock is possible. This method is invoked
under metaStorageMux and then master key change log acquired, but in
doChangeMasterKey() master key change lock acquired first and then
metaStorageMux.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/pagemem/wal/record/WALRecord.java
##########
@@ -233,7 +233,25 @@
TRACKING_PAGE_REPAIR_DELTA(61, PHYSICAL),
/** Atomic out-of-order update. */
- OUT_OF_ORDER_UPDATE(62, LOGICAL);
+ OUT_OF_ORDER_UPDATE(62, LOGICAL),
+
+ /** Encrypted WAL-record. */
+ ENCRYPTED_RECORD_V2(63, PHYSICAL),
+
+ /** Ecnrypted data record. */
+ ENCRYPTED_DATA_RECORD_V2(64, LOGICAL),
+
+ /** Master key change record containing multiple keys for single cache
group. */
+ MASTER_KEY_CHANGE_RECORD_V2(65, LOGICAL),
+
+ /** Logical record to restart reencryption with the latest encryption
key. */
+ REENCRYPTION_START_RECORD(66, LOGICAL),
+
+ /** Partition meta page delta record includes encryption status data.
*/
+ PARTITION_META_PAGE_UPDATE_COUNTERS_V3(67, PHYSICAL),
Review comment:
Maybe `PARTITION_META_PAGE_DELTA_RECORD_V3`? WDYT?
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/encryption/GridEncryptionManager.java
##########
@@ -483,16 +534,18 @@ public void onLocalJoin() {
if (dataBag.isJoiningNodeClient() ||
dataBag.commonDataCollectedFor(ENCRYPTION_MGR.ordinal()))
return;
- HashMap<Integer, byte[]> knownEncKeys = knownEncryptionKeys();
+ HashMap<Integer, GroupKeyEncrypted> knownEncKeys = grpKeys.getAll();
Review comment:
As far as I understand master key can be changed concurrently with this
method and getAll() encrypt keys with master key, so we should use
grpKeys.getAll() under master key change read lock (please pay attention to
locks order to avoid deadlocks). At least in this method, perhaps in other
methods too (please review also usages of getAll(grpId) and setGroupKeys
methods, perhaps these methods should be invoked under lock too)
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
##########
@@ -2863,11 +2869,16 @@ private RestoreLogicalState applyLogicalUpdates(
break;
- case MASTER_KEY_CHANGE_RECORD:
+ case MASTER_KEY_CHANGE_RECORD_V2:
Review comment:
Let's also keep `case MASTER_KEY_CHANGE_RECORD:`
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/managers/encryption/GridEncryptionManager.java
##########
@@ -811,6 +1124,53 @@ private void
sendGenerateEncryptionKeyRequest(GenerateEncryptionKeyFuture fut) t
ctx.io().sendToGridTopic(rndNode.id(), TOPIC_GEN_ENC_KEY, req,
SYSTEM_POOL);
}
+ /**
+ * @param grpIds Cache group IDs.
+ * @throws IgniteCheckedException If failed.
+ */
+ private void startReencryption(Collection<Integer> grpIds) throws
IgniteCheckedException {
+ if (pageScanner.disabled())
+ return;
+
+ for (int grpId : grpIds) {
+ IgniteInternalFuture<?> fut = pageScanner.schedule(grpId);
+
+ fut.listen(f -> {
+ try {
+ f.get();
Review comment:
Because exception driven flow control is an anti-pattern
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
