Copilot commented on code in PR #61922:
URL: https://github.com/apache/doris/pull/61922#discussion_r3014774358
##########
fe/fe-core/src/main/java/org/apache/doris/persist/gson/GsonUtils.java:
##########
@@ -932,6 +932,9 @@ public void write(JsonWriter out, T value) throws
IOException {
((GsonPreProcessable) value).gsonPreProcess();
}
delegate.write(out, value);
+ if (value instanceof GsonPreProcessable) {
+ ((GsonPreProcessable) value).gsonPostSerialize();
+ }
Review Comment:
In PreProcessTypeAdapterFactory.write(), gsonPostSerialize() is only invoked
after delegate.write() returns successfully. If delegate.write() throws, the
transient state set by gsonPreProcess() (e.g.,
CloudReplica.primaryClusterToBackend snapshot) will remain on the object,
potentially increasing memory usage / causing incorrect subsequent
serialization. Consider wrapping delegate.write(out, value) in a try/finally
and calling gsonPostSerialize() from the finally block (only if
gsonPreProcess() ran).
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTabletInvertedIndex.java:
##########
@@ -35,6 +40,14 @@ public class CloudTabletInvertedIndex extends
TabletInvertedIndex {
// for cloud mode, no need to know the replica's backend
private Long2ObjectOpenHashMap<Replica> replicaMetaMap = new
Long2ObjectOpenHashMap<>();
+ // Centralized cluster-to-BE mappings (moved from per-CloudReplica
ConcurrentHashMaps).
+ // Outer key: clusterId (typically 1-3 clusters). Inner key: replicaId.
Value: beId or timestamp.
+ private final ConcurrentHashMap<String, ConcurrentLong2LongHashMap>
clusterPrimaryBeMap
+ = new ConcurrentHashMap<>();
+ // Stores [beId, timestamp] atomically per replicaId to avoid TOCTOU races
between separate maps.
+ private final ConcurrentHashMap<String,
ConcurrentLong2ObjectHashMap<long[]>> clusterSecondaryMap
+ = new ConcurrentHashMap<>();
Review Comment:
PR description mentions adding "3 central ... maps" in
CloudTabletInvertedIndex, but the implementation here introduces 2 central maps
(clusterPrimaryBeMap and clusterSecondaryMap). Please update the PR description
to match the actual change, or add the missing third map if it was intended.
--
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]
