imbajin commented on code in PR #2906:
URL: 
https://github.com/apache/incubator-hugegraph/pull/2906#discussion_r2531932015


##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -370,50 +372,6 @@ private boolean peerEquals(PeerId p1, PeerId p2) {
     }
 
     private Replicator.State getReplicatorState(PeerId peerId) {
-        var replicateGroup = getReplicatorGroup();
-        if (replicateGroup == null) {
-            return null;
-        }
-
-        ThreadId threadId = replicateGroup.getReplicator(peerId);
-        if (threadId == null) {
-            return null;
-        } else {
-            Replicator r = (Replicator) threadId.lock();
-            if (r == null) {
-                return Replicator.State.Probe;
-            }
-            Replicator.State result = getState(r);
-            threadId.unlock();
-            return result;
-        }
-    }
-
-    private ReplicatorGroup getReplicatorGroup() {
-        var clz = this.raftNode.getClass();
-        try {
-            var f = clz.getDeclaredField("replicatorGroup");
-            f.setAccessible(true);
-            var group = (ReplicatorGroup) f.get(this.raftNode);
-            f.setAccessible(false);
-            return group;
-        } catch (NoSuchFieldException | IllegalAccessException e) {
-            log.info("getReplicatorGroup: error {}", e.getMessage());
-            return null;
-        }
-    }
-
-    private Replicator.State getState(Replicator r) {
-        var clz = r.getClass();
-        try {
-            var f = clz.getDeclaredField("state");
-            f.setAccessible(true);
-            var state = (Replicator.State) f.get(this.raftNode);
-            f.setAccessible(false);
-            return state;
-        } catch (NoSuchFieldException | IllegalAccessException e) {
-            log.info("getReplicatorGroup: error {}", e.getMessage());
-            return null;
-        }
+        return RaftReflectionUtil.getReplicatorState(this.raftNode, peerId);

Review Comment:
   💡 **推荐的简洁优雅方案**
   
   考虑到该 PR 的核心问题是修复 issue #2867 中的反射参数错误,有两个更好的实现方案:
   
   ---
   
   ### 方案 A: 最小改动 (推荐用于快速修复)
   直接在 RaftEngine.java 和 PartitionEngine.java 中修复 bug + 改进 finally:
   
   ```java
   private Replicator.State getState(Replicator r) {
       var clz = r.getClass();
       try {
           var f = clz.getDeclaredField("state");
           f.setAccessible(true);
           try {
               return (Replicator.State) f.get(r); // 修复: 使用 r 而不是 this.raftNode
           } finally {
               f.setAccessible(false);
           }
       } catch (NoSuchFieldException | IllegalAccessException e) {
           log.info("getState: error {}", e.getMessage());
           return null;
       }
   }
   ```
   
   **优点**: 
   - 零依赖变更
   - 直接修复 bug
   - 改进资源管理
   
   ---
   
   ### 方案 B: 代码复用 (推荐用于长期维护)
   将 RaftReflectionUtil 放到 hg-pd-core 而不是 hugegraph-common:
   
   1. 在 
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/util/RaftReflectionUtil.java
 创建工具类
   2. hg-store-core 添加对 hg-pd-core 的依赖
   3. 两个模块共享同一个工具类实现
   
   **优点**:
   - 消除代码重复
   - 依赖关系更合理 (store 依赖 pd-core,而不是污染 common 模块)
   - jraft-core 依赖限制在真正需要的模块内
   
   ---
   
   **结论**: 建议采用方案 A 用于快速修复 bug,或方案 B 如果希望长期消除重复代码。当前方案在架构上不够合理。



-- 
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]

Reply via email to