Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add zk register code #355

Merged
merged 1 commit into from
Feb 9, 2020
Merged

Conversation

pantianying
Copy link
Member

What this PR does:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


@codecov-io
Copy link

Codecov Report

Merging #355 into 1.3 will increase coverage by 1.2%.
The diff coverage is 63.28%.

Impacted file tree graph

@@            Coverage Diff            @@
##              1.3     #355     +/-   ##
=========================================
+ Coverage   66.68%   67.88%   +1.2%     
=========================================
  Files         118      118             
  Lines        7419     7099    -320     
=========================================
- Hits         4947     4819    -128     
+ Misses       1981     1809    -172     
+ Partials      491      471     -20
Impacted Files Coverage Δ
remoting/etcdv3/client.go 57.8% <ø> (ø) ⬆️
remoting/etcdv3/listener.go 46.08% <ø> (ø) ⬆️
remoting/zookeeper/facade.go 74.28% <0%> (ø) ⬆️
remoting/etcdv3/facade.go 0% <0%> (ø) ⬆️
registry/etcdv3/registry.go 73.52% <100%> (+22.01%) ⬆️
config_center/zookeeper/impl.go 40% <25.92%> (+0.41%) ⬆️
remoting/zookeeper/listener.go 52.02% <30.76%> (+3.81%) ⬆️
registry/etcdv3/listener.go 76.74% <50%> (ø) ⬆️
registry/zookeeper/listener.go 60.86% <66.66%> (ø) ⬆️
remoting/zookeeper/client.go 68.06% <80.95%> (+3.61%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b13a70...7fcb34e. Read the comment docs.

@pantianying pantianying requested review from AlexStocks, hxmhlt and fangyincheng and removed request for hxmhlt February 8, 2020 13:30
rawURL string
encodedURL string
dubboPath string
//conf config.URL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if useless then delete

func (r *BaseRegistry) register(c common.URL) error {
var (
err error
//revision string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if useless then delete


params.Add("pid", processID)
params.Add("ip", localIP)
//params.Add("timeout", fmt.Sprintf("%d", int64(r.Timeout)/1e6))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if useless then delete

// Dubbo java consumer to start looking for the provider url,because the category does not match,
// the provider will not find, causing the consumer can not start, so we use consumers.
// DubboRole = [...]string{"consumer", "", "", "provider"}
// params.Add("category", (RoleType(PROVIDER)).Role())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if useless then delete

)

// ZookeeperClient ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comment for this struct

)

import (
"github.com/apache/dubbo-go/common/logger"
"github.com/apache/dubbo-go/remoting"
)

// ZkEventListener ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comment for this struct

)

// Options ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comment for this struct

@@ -36,18 +36,23 @@ import (
zk "github.com/apache/dubbo-go/remoting/zookeeper"
)

// RegistryDataListener ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comment for this file

return perrors.Errorf("@c{%v} type is not referencer or provider", c)
}
encodedURL = url.QueryEscape(rawURL)
dubboPath = strings.ReplaceAll(dubboPath, "$", "%24")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why not use url.QueryEscape for dubboPath?

logger.Errorf("facadeBasedRegistry.CreatePath(path{%s}) = error{%#v}", dubboPath, perrors.WithStack(err))
return "", "", perrors.WithMessagef(err, "facadeBasedRegistry.CreatePath(path:%s)", dubboPath)
}
params.Add("anyhost", "true")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean?


params.Add("side", (common.RoleType(common.PROVIDER)).Role())

if len(c.Methods) == 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if len(c.Methods) != 0 { ...}????

}
dubboPath = fmt.Sprintf("/dubbo/%s/%s", r.service(c), common.DubboNodes[common.PROVIDER])
r.cltLock.Lock()
err = r.facadeBasedRegistry.CreatePath(dubboPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should we create provider's path here?


params.Add("protocol", c.Protocol)
params.Add("category", (common.RoleType(common.CONSUMER)).String())
params.Add("dubbo", "dubbogo-consumer-"+constant.Version)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

params.Add("dubbo", "dubbo-provider-golang-"+constant.Version) but here is lack of golang. Is that right??


// sleepWait...
func sleepWait(n int) {
wait := time.Duration((n + 1) * 2e8)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2e8 is hard to understand, please define a constant.

return
}
logger.Warnf("getListener() = err:%v", perrors.WithStack(err))
time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just define the RegistryConnDelay as time.Duration(3* time.Second)

remoting/zookeeper/listener.go Show resolved Hide resolved
@fangyincheng fangyincheng merged commit 5f82281 into apache:1.3 Feb 9, 2020
@pantianying pantianying deleted the fix_zkproblemto1.3 branch February 24, 2020 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants