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 polaris subscribe #2100

Merged
merged 19 commits into from
Nov 11, 2022
Merged

add polaris subscribe #2100

merged 19 commits into from
Nov 11, 2022

Conversation

jasondeng1997
Copy link
Member

What this PR does:

Which issue(s) this PR fixes:
Fixes #

You should pay attention to items below to ensure your pr passes our ci test
We do not merge pr with ci tests failed

  • All ut passed (run 'go test ./...' in project root)
  • After go-fmt ed , run 'go fmt project' using goland.
  • Golangci-lint passed, run 'sudo golangci-lint run' in project root.
  • Your new-created file needs to have apache license at the top, like other existed file does.
  • All integration test passed. You can run integration test locally (with docker env). Clone our dubbo-go-samples project and replace the go.mod to your dubbo-go, and run 'sudo sh start_integration_test.sh' at root of samples project root. (M1 Slice is not Support)

@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2022

Codecov Report

Merging #2100 (fcfbef8) into 3.0 (5903520) will decrease coverage by 0.20%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              3.0    #2100      +/-   ##
==========================================
- Coverage   44.74%   44.53%   -0.21%     
==========================================
  Files         281      282       +1     
  Lines       16864    16941      +77     
==========================================
- Hits         7546     7545       -1     
- Misses       8527     8606      +79     
+ Partials      791      790       -1     
Impacted Files Coverage Δ
cluster/cluster/base/cluster_invoker.go 24.44% <0.00%> (-13.34%) ⬇️
protocol/grpc/client.go 63.38% <0.00%> (-9.96%) ⬇️
protocol/grpc/server.go 68.23% <0.00%> (-8.91%) ⬇️
config/service_config.go 52.09% <0.00%> (-0.96%) ⬇️
config/reference_config.go 31.03% <0.00%> (-0.95%) ⬇️
config/protocol_config.go 50.00% <0.00%> (ø)
config/tls_config.go 0.00% <0.00%> (ø)
metrics/prometheus/reporter.go 31.79% <0.00%> (+1.53%) ⬆️
filter/metrics/filter.go 100.00% <0.00%> (+15.00%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -67,11 +68,13 @@ func newPolarisRegistry(url *common.URL) (registry.Registry, error) {
}

type polarisRegistry struct {
consumer api.ConsumerAPI
namespace string
Copy link
Contributor

Choose a reason for hiding this comment

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

这个 ns 信息在哪里初始化呢?

})
}
registry.NewServiceInstancesChangedEvent(serviceName, dubboInstances)
listener.Next()
Copy link
Contributor

Choose a reason for hiding this comment

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

你这里不吧事件传递给 listener 吗?你的把事件塞到 chan 里面去呀,不然你下面的 listener.Next 等啥呢?

Copy link
Member Author

Choose a reason for hiding this comment

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

这个事件需要传递给listener吗,这个是一个notify事件,执行通知就行了

Copy link
Member Author

Choose a reason for hiding this comment

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

image

Copy link
Member Author

Choose a reason for hiding this comment

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

例如nacos

Copy link
Contributor

Choose a reason for hiding this comment

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

你先看下 nacos.subscribe 里面做的事情

Copy link
Contributor

Choose a reason for hiding this comment

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

你先看下 nacos.subscribe 里面做的事情,再看看你这里,你都没有吧 event 传到 listener 里面 chan,你这里 next 不就是死等吗


// Next returns next service event once received
func (pl *polarisListener) Next() (*registry.ServiceEvent, error) {
for {
select {
case <-pl.closeCh:
logger.Warnf("polaris listener is close!listenUrl:%+v", pl.listenUrl)
logger.Warnf("polaris listener is close!listenUrl:%+v")
Copy link
Contributor

Choose a reason for hiding this comment

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

这个日志打印调整下

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@@ -93,3 +82,5 @@ consumer:
plugin:
subscribeLocalChannel:
channelBufferSize: 50
statReporter:
Copy link
Contributor

Choose a reason for hiding this comment

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

放错位置了,就在原来的基础上改,不要调整 statReporter 的位置

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@jasondeng1997
Copy link
Member Author

都改了

@AlexStocks AlexStocks merged commit 188b993 into apache:3.0 Nov 11, 2022
@justxuewei justxuewei added this to the v3.0.4 milestone Dec 4, 2022
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.

6 participants