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

channelz: implement GetServer method #2537

Merged
merged 2 commits into from
Jan 9, 2019
Merged

Conversation

kazegusuri
Copy link
Contributor

GetServer is not implemented for now.

And also fixes GetChannel, GetSubchannel, GetSocket response when requested id not found.
By gRFC spec, it should return NotFound status code, not empty message with OK.

@@ -155,6 +155,15 @@ func GetSocket(id int64) *SocketMetric {
return db.get().GetSocket(id)
}

// GetServer returns the ServerMetric for the server (identified by id).
Copy link
Contributor

@lyuxuan lyuxuan Dec 27, 2018

Choose a reason for hiding this comment

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

db.get().GetServers(id, 1) may not get the result for server with the specified id. It will get the first server with id>= specified id. I think you need to implement a new function db.get().GetServer(id) on the channelMap struct, which will find the server with the specified id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will get the first server with id>= specified id.

Yes, that's right. So I checks returned server matches the specified id's server.

I think you need to implement a new function db.get().GetServer(id) on the channelMap struct, which will find the server with the specified id.

OK. it's easy to understand than the current implementation. I will add the function.

@lyuxuan
Copy link
Contributor

lyuxuan commented Jan 3, 2019

friendly ping?

@kazegusuri
Copy link
Contributor Author

Addressed a comment. Please take a look.


ss, _ := channelz.GetServers(0, 0)
if len(ss) != 1 {
t.Fatalf("there should only be one servers, not %d", len(ss))
Copy link
Contributor

Choose a reason for hiding this comment

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

s/servers/server

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed!

@lyuxuan lyuxuan merged commit 8fd063a into grpc:master Jan 9, 2019
@kazegusuri kazegusuri deleted the impl-get-server branch January 10, 2019 04:36
@lyuxuan lyuxuan added the Type: Feature New features or improvements in behavior label Jan 10, 2019
@lyuxuan lyuxuan added this to the 1.18 Release milestone Jan 10, 2019
@dfawley dfawley changed the title Implement GetServer method for channelz channelz: implement GetServer method Jan 15, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jul 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants