robocanic commented on code in PR #1423:
URL: https://github.com/apache/dubbo-admin/pull/1423#discussion_r2901861710


##########
pkg/core/discovery/component.go:
##########
@@ -111,10 +116,65 @@ func (d *discoveryComponent) Init(ctx 
runtime.BuilderContext) error {
        if err != nil {
                return err
        }
+
+       // Initialize leader election if using DB store
+       if ctx.Config().Store.Type != storecfg.Memory {

Review Comment:
   Suggestion: 这里的多个if else处理可以更优雅一点,不符合预期的情况提前返回。
   比如 
   if ctx.Config().Store.Type == storecfg.Memory   return
   // 再写后面的逻辑
   



##########
pkg/core/store/component.go:
##########
@@ -109,3 +122,29 @@ func (sc *storeComponent) ResourceKindRoute(k 
coremodel.ResourceKind) (ResourceS
        return nil, fmt.Errorf("%s is not supported by store yet", k)
 
 }
+
+// GetDB returns the shared DB connection if the underlying store is DB-backed
+// Implements the leader.DBSource interface
+func (sc *storeComponent) GetDB() (*gorm.DB, bool) {
+       // Try to get DB from any store that has a Pool() method (all 
GormStores share the same ConnectionPool)
+       for _, store := range sc.stores {
+               if pp, ok := store.(poolProvider); ok {

Review Comment:
   Suggestion: 这个for-loop里面的if 条件不满足的可以先continue,编码看起来会更优雅一点



##########
pkg/core/leader/leader.go:
##########
@@ -0,0 +1,279 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package leader
+
+import (
+       "context"
+       "fmt"
+       "os"
+       "sync/atomic"
+       "time"
+
+       "github.com/google/uuid"
+       "gorm.io/gorm"
+
+       "github.com/apache/dubbo-admin/pkg/core/logger"
+)
+
+const (
+       // DefaultLeaseDuration is the default duration for a leader lease
+       DefaultLeaseDuration = 30 * time.Second
+       // DefaultRenewInterval is the default interval for renewing the lease
+       DefaultRenewInterval = 10 * time.Second

Review Comment:
   Question: 
这里renewInterval和leaseDuration有整除的关系,会不会出现这样一种情况,某一个正常的节点在上一个周期是主节点,但renew时由于并发的关系被其他节点抢占了主,从而该节点变成了从节点,但是由主->从的这个状态变化并没有让这个节点的discovery和engine停止,下一个周期会有两个节点的discovery和engine都在do
 list-watch,往数据库里面写数据,会导致脏数据的产生。
   这里面有两个核心问题:
   1. 一个是已经成为主节点的节点,在正常情况下应该一直持有这个lease,除非自己挂了,才需要重新选主
   2. 如果有异常情况,主节点虽然是正常的,但是由于网络原因在下一个周期没抢到lease,那该节点需要停止主节点才能干的事情



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