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

grpc bump: Implement master/client/v3/naming new endpointManager & resolver #12652

Closed
ptabor opened this issue Jan 30, 2021 · 5 comments · Fixed by #12669 or #12709
Closed

grpc bump: Implement master/client/v3/naming new endpointManager & resolver #12652

ptabor opened this issue Jan 30, 2021 · 5 comments · Fixed by #12669 or #12709

Comments

@ptabor
Copy link
Contributor

ptabor commented Jan 30, 2021

Naming

In order to migrate out of decommissioned in grpc-1.30 APIs of naming (and picker),
still addressing the use-cases documented in:
https://github.com/etcd-io/etcd/blob/master/Documentation/dev-guide/grpc_naming.md

We execute following plan:

  1. Introduce new interfaces as defined in: Replacement API for client/v3/naming package to be compatible with new GRPC1.30+ resolver API. #12614
  2. Implement them [help needed]
  3. Migrate grpc proxy on the new implementation. This is hopefully ready:(ptabor@142b9a3)

Picker

Apart of dealing with clientv3/naming as documented above,
we need to migrate clientv3/balancer on a new picker.
The code from contributions like:
#12384
#12636
#12439
Might be good for this (when isolated and passing test).
Mid term I would think why/whether etcd needs own balancer and why just:
https://pkg.go.dev/google.golang.org/grpc@v1.35.0/balancer/roundrobin
on top of some 'passthrough' like https://pkg.go.dev/google.golang.org/grpc/resolver

ptabor added a commit to ptabor/etcd that referenced this issue Jan 30, 2021
This is not yet implementation, just API and tests to be filled
with implementation in next CLs,
tracked by: etcd-io#12652

We propose here 3 packages:
 - clientv3/naming/endpoints ->
    That is abstraction layer over etcd that allows to write, read &
    watch Endpoints information. It's independent from GRPC API. It hides
    the storage details.

 - clientv3/naming/endpoints/internal ->
    That contains the grpc's compatible Update class to preserve the
    internal JSON mashalling format.

 - clientv3/naming/resolver ->
   That implements the GRPC resolver API, such that etcd can be
   used for connection.Dial in grpc.

Please see the grpc_naming.md document changes & grpcproxy/cluster.go
new integration, to see how the new abstractions work.
ptabor added a commit to ptabor/etcd that referenced this issue Jan 30, 2021
This is not yet implementation, just API and tests to be filled
with implementation in next CLs,
tracked by: etcd-io#12652

We propose here 3 packages:
 - clientv3/naming/endpoints ->
    That is abstraction layer over etcd that allows to write, read &
    watch Endpoints information. It's independent from GRPC API. It hides
    the storage details.

 - clientv3/naming/endpoints/internal ->
    That contains the grpc's compatible Update class to preserve the
    internal JSON mashalling format.

 - clientv3/naming/resolver ->
   That implements the GRPC resolver API, such that etcd can be
   used for connection.Dial in grpc.

Please see the grpc_naming.md document changes & grpcproxy/cluster.go
new integration, to see how the new abstractions work.
@ptabor ptabor changed the title Implement master/client/v3/naming new endpointManager & resolver grpc bump: Implement master/client/v3/naming new endpointManager & resolver Feb 1, 2021
@lemonlinger
Copy link

I'd like to claim this task. Is there a deadline for this?

@ptabor
Copy link
Contributor Author

ptabor commented Feb 2, 2021

@lemonlinger Thank you. Many users are waiting for moving that dependency forward, so I hope to have it completed in O(2) weeks. We can decouple this into smaller pieces:

  1. [Done in Implement Update method for EndpointManager #12667] Write variant of the resolver_test(

    conn, err := grpc.Dial("etc://foo", grpc.WithResolvers(etcdResolver))
    ) that uses the old API: https://etcd.io/docs/v3.3.12/dev-guide/grpc_naming/

  2. [Done in Implement Update method for EndpointManager #12667] EndpointManager add/delete (

    func (m *endpointManager) Update(ctx context.Context, updates []*UpdateWithOpts) error {
    ) implementation

  3. [Done in Implement Update method for EndpointManager #12667] Write variant of the resolver_test(

    conn, err := grpc.Dial("etc://foo", grpc.WithResolvers(etcdResolver))
    ) that uses new endpointManager, but old resolver code

  4. Implementation of new Endpoint Watch (& List DONE]

  5. Resolver on top of Endpoint Watch

  6. Final resolver_test that entirely uses new API.

I think 1&2 can be done in parallel and its better to claim smaller pieces of work.

@lemonlinger
Copy link

@peterbourgon OK

@ptabor
Copy link
Contributor Author

ptabor commented Feb 8, 2021

3 more PRs to merge:

And that's a wrap.

@ptabor
Copy link
Contributor Author

ptabor commented Feb 10, 2021

In my fork (https://github.com/ptabor/etcd/tree/20210210-grpc-bump) I have version with the PRs applies + 1 test fix + grpc-1.32 bump that passes all the tests

@ptabor ptabor linked a pull request Feb 23, 2021 that will close this issue
@chaochn47 chaochn47 mentioned this issue Oct 17, 2023
24 tasks
chaochn47 pushed a commit to chaochn47/etcd that referenced this issue Oct 18, 2023
This is not yet implementation, just API and tests to be filled
with implementation in next CLs,
tracked by: etcd-io#12652

We propose here 3 packages:
 - clientv3/naming/endpoints ->
    That is abstraction layer over etcd that allows to write, read &
    watch Endpoints information. It's independent from GRPC API. It hides
    the storage details.

 - clientv3/naming/endpoints/internal ->
    That contains the grpc's compatible Update class to preserve the
    internal JSON mashalling format.

 - clientv3/naming/resolver ->
   That implements the GRPC resolver API, such that etcd can be
   used for connection.Dial in grpc.

Please see the grpc_naming.md document changes & grpcproxy/cluster.go
new integration, to see how the new abstractions work.
chaochn47 pushed a commit to chaochn47/etcd that referenced this issue Oct 18, 2023
This is not yet implementation, just API and tests to be filled
with implementation in next CLs,
tracked by: etcd-io#12652

We propose here 3 packages:
 - clientv3/naming/endpoints ->
    That is abstraction layer over etcd that allows to write, read &
    watch Endpoints information. It's independent from GRPC API. It hides
    the storage details.

 - clientv3/naming/endpoints/internal ->
    That contains the grpc's compatible Update class to preserve the
    internal JSON mashalling format.

 - clientv3/naming/resolver ->
   That implements the GRPC resolver API, such that etcd can be
   used for connection.Dial in grpc.

Please see the grpc_naming.md document changes & grpcproxy/cluster.go
new integration, to see how the new abstractions work.

Signed-off-by: Chao Chen <chaochn@amazon.com>
chaochn47 pushed a commit to chaochn47/etcd that referenced this issue Oct 18, 2023
This is not yet implementation, just API and tests to be filled
with implementation in next CLs,
tracked by: etcd-io#12652

We propose here 3 packages:
 - clientv3/naming/endpoints ->
    That is abstraction layer over etcd that allows to write, read &
    watch Endpoints information. It's independent from GRPC API. It hides
    the storage details.

 - clientv3/naming/endpoints/internal ->
    That contains the grpc's compatible Update class to preserve the
    internal JSON mashalling format.

 - clientv3/naming/resolver ->
   That implements the GRPC resolver API, such that etcd can be
   used for connection.Dial in grpc.

Please see the grpc_naming.md document changes & grpcproxy/cluster.go
new integration, to see how the new abstractions work.

Signed-off-by: Chao Chen <chaochn@amazon.com>
chaochn47 pushed a commit to chaochn47/etcd that referenced this issue Oct 18, 2023
This is not yet implementation, just API and tests to be filled
with implementation in next CLs,
tracked by: etcd-io#12652

We propose here 3 packages:
 - clientv3/naming/endpoints ->
    That is abstraction layer over etcd that allows to write, read &
    watch Endpoints information. It's independent from GRPC API. It hides
    the storage details.

 - clientv3/naming/endpoints/internal ->
    That contains the grpc's compatible Update class to preserve the
    internal JSON mashalling format.

 - clientv3/naming/resolver ->
   That implements the GRPC resolver API, such that etcd can be
   used for connection.Dial in grpc.

Please see the grpc_naming.md document changes & grpcproxy/cluster.go
new integration, to see how the new abstractions work.

Signed-off-by: Chao Chen <chaochn@amazon.com>
chaochn47 pushed a commit to chaochn47/etcd that referenced this issue Oct 19, 2023
This is not yet implementation, just API and tests to be filled
with implementation in next CLs,
tracked by: etcd-io#12652

We propose here 3 packages:
 - clientv3/naming/endpoints ->
    That is abstraction layer over etcd that allows to write, read &
    watch Endpoints information. It's independent from GRPC API. It hides
    the storage details.

 - clientv3/naming/endpoints/internal ->
    That contains the grpc's compatible Update class to preserve the
    internal JSON mashalling format.

 - clientv3/naming/resolver ->
   That implements the GRPC resolver API, such that etcd can be
   used for connection.Dial in grpc.

Please see the grpc_naming.md document changes & grpcproxy/cluster.go
new integration, to see how the new abstractions work.

Signed-off-by: Chao Chen <chaochn@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants