sk0x50 commented on a change in pull request #7941:
URL: https://github.com/apache/ignite/pull/7941#discussion_r501726235
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/pagemem/wal/record/EncryptedRecord.java
##########
@@ -43,7 +43,7 @@ public EncryptedRecord(int grpId, RecordType plainRecType) {
/** {@inheritDoc} */
@Override public RecordType type() {
- return RecordType.ENCRYPTED_RECORD;
+ return RecordType.ENCRYPTED_RECORD_V2;
Review comment:
Could you please describe in short why you need to change the type of
this WAL record?
WAL record was not changed itself, so it looks weird to me.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/pagemem/wal/record/EncryptedRecord.java
##########
@@ -43,7 +43,7 @@ public EncryptedRecord(int grpId, RecordType plainRecType) {
/** {@inheritDoc} */
@Override public RecordType type() {
- return RecordType.ENCRYPTED_RECORD;
+ return RecordType.ENCRYPTED_RECORD_V2;
Review comment:
> (see change in RecordDataV1Serializer.readEncryptedData)
Yep, I saw this, but it does not explain why you modify the existing WAL
record.
In my understanding, WAL records must be immutable in order to avoid
compatibility issues in the future.
I would prefer adding a new WAL record (it can extend an existing one).
Perhaps, RecordDataV1Serializer implementation will be cleaner in that case.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/pagemem/wal/record/EncryptedRecord.java
##########
@@ -43,7 +43,7 @@ public EncryptedRecord(int grpId, RecordType plainRecType) {
/** {@inheritDoc} */
@Override public RecordType type() {
- return RecordType.ENCRYPTED_RECORD;
+ return RecordType.ENCRYPTED_RECORD_V2;
Review comment:
> (see change in RecordDataV1Serializer.readEncryptedData)
Yep, I saw this, but it does not explain why you modify the existing WAL
record.
In my understanding, WAL records must be immutable in order to avoid
compatibility issues in the future.
I would prefer adding a new WAL record (it can extend an existing one).
Perhaps, RecordDataV1Serializer implementation will be cleaner in that case.
----------------------------------------------------------------
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]
