NeverENG commented on code in PR #3373:
URL: https://github.com/apache/dubbo-go/pull/3373#discussion_r3369039548


##########
metadata/report/nacos/report.go:
##########
@@ -165,30 +162,42 @@ func (n *nacosMetadataReport) 
RegisterServiceAppMapping(key string, group string
                DataId: key,
                Group:  group,
        })
-       if oldVal != "" {
-               oldApps := strings.Split(oldVal, constant.CommaSeparator)
-               if len(oldApps) > 0 {
-                       for _, app := range oldApps {
-                               if app == value {
-                                       return nil
-                               }
-                       }
-               }
-               value = oldVal + constant.CommaSeparator + value
+       merged, changed := report.MergeServiceAppMapping(oldVal, value)
+       if !changed {
+               return nil
        }
-       return n.storeMetadata(vo.ConfigParam{
+       param := vo.ConfigParam{
                DataId:  key,
                Group:   group,
-               Content: value,
-       })
+               Content: merged,
+       }
+       if oldVal != "" {
+               // CasMd5 is an optimistic UPDATE: Nacos publishes only if the 
server content still
+               // matches what we read, detecting concurrent appends. It 
cannot guard the first INSERT
+               // (Nacos has no create-if-absent), so the initial concurrent 
registration of a
+               // brand-new interface can still race. This is a known 
Nacos-only limitation; the
+               // etcd and zookeeper reports do not have it.
+               param.CasMd5 = fmt.Sprintf("%x", md5.Sum([]byte(oldVal)))
+       }
+       if err := n.storeMetadata(param); err != nil {
+               if param.CasMd5 != "" {
+                       // Nacos surfaces a CAS rejection and a transport error 
the same way, so they
+                       // cannot be told apart here. Treat the failure as a 
retriable conflict rather
+                       // than risk dropping a real concurrent update; the 
underlying error is preserved
+                       // for diagnosis.
+                       return fmt.Errorf("publish mapping %s (%v): %w", key, 
err, report.ErrMappingCASConflict)
+               }
+               return err
+       }
+       return nil
 }

Review Comment:
   问题已解决
   换算法应该不影响,#3370 是 app+revision -> MetaDataInfo 的映射。#3373(本PR)是 
service-app-mapping,interface 到 app 到映射,这里的 md5 是 nacos 为实现 CAS 乐观锁的指纹,修改 #3370 
对本 pr 无影响



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