Copilot commented on code in PR #59815:
URL: https://github.com/apache/doris/pull/59815#discussion_r2685179099
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTablet.java:
##########
@@ -96,4 +106,18 @@ public void addReplica(Replica replica, boolean isRestore) {
}
}
+ @Override
+ public void gsonPostProcess() throws IOException {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("convert replica to replicas for CloudTablet: {},
replica: {}, replicas: {}", this.id,
+ this.replica, this.replicas);
+ }
+ if (replica != null) {
+ if (this.replicas == null) {
+ this.replicas = Lists.newArrayList();
+ }
+ this.replicas.add(replica);
+ this.replica = null;
+ }
+ }
Review Comment:
The new gsonPostProcess method that handles backward compatibility from
version 4.1 to 4.0 lacks test coverage. Consider adding a test similar to the
TabletTest.testSerialization method that verifies:
1. Deserialization of CloudTablet with the old 'replica' field (from v4.1)
correctly populates the 'replicas' list
2. The 'replica' field is properly nulled out after migration
3. Edge cases like null replica, null replicas list, etc. are handled
correctly
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTablet.java:
##########
@@ -96,4 +106,18 @@ public void addReplica(Replica replica, boolean isRestore) {
}
}
+ @Override
+ public void gsonPostProcess() throws IOException {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("convert replica to replicas for CloudTablet: {},
replica: {}, replicas: {}", this.id,
+ this.replica, this.replicas);
+ }
+ if (replica != null) {
+ if (this.replicas == null) {
+ this.replicas = Lists.newArrayList();
+ }
+ this.replicas.add(replica);
Review Comment:
Consider checking if the replica already exists in the replicas list before
adding it, to guard against potential duplicate entries. While the normal
deserialization path from 4.1 should only populate the 'replica' field,
defensive programming would prevent issues if both fields are somehow present
in the serialized data.
```suggestion
this.replicas.add(replica);
} else if (!this.replicas.contains(replica)) {
this.replicas.add(replica);
}
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
