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

LoadBalancer and ConnectionFactory: add a context of the caller #2168

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

idelpivnitskiy
Copy link
Member

Motivation:

LoadBalancer and ConnectionFactory API is disconnected from the
caller, lacks context who/why selects/creates a connection, and has no
API to propagate additional information from the caller (request).

Modifications:

  • LoadBalancer: add new selectConnection(Predicate, ContextMap)
    method, deprecate selectConnection(Predicate);
  • ConnectionFactory: add new
    newConnection(Address, ContextMap, TransportObserver) method,
    deprecate newConnection(Address, TransportObserver);
  • Migrate all LB/CF implementations and tests to use new API;
  • Add DeprecatedToNewConnectionFactoryFilter after each user-defined
    CF to make sure old/new filters work in the same pipeline;

Result:

Information can be propagated from the caller to the
LoadBalancer and ConnectionFactory.

Motivation:

`LoadBalancer` and `ConnectionFactory` API is disconnected from  the
caller, lacks context who/why selects/creates a connection, and has no
API to propagate additional information from the caller (request).

Modifications:

- `LoadBalancer`: add new `selectConnection(Predicate, ContextMap)`
method, deprecate `selectConnection(Predicate)`;
- `ConnectionFactory`: add new
`newConnection(Address, ContextMap, TransportObserver)` method,
deprecate `newConnection(Address, TransportObserver)`;
- Migrate all LB/CF implementations and tests to use new API;
- Add `DeprecatedToNewConnectionFactoryFilter` after each user-defined
CF to make sure old/new filters work in the same pipeline;

Result:

Information can be propagated from the caller to the
`LoadBalancer` and `ConnectionFactory`.
@chemicL chemicL merged commit 4141e33 into apple:main Apr 13, 2022
@idelpivnitskiy idelpivnitskiy deleted the lb-cf-context branch April 13, 2022 15:23
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request Jul 22, 2022
Motivation:

apple#2168 add `ContextMap` for `LoadBalancer` and `ConnectionFactory`, but
does not let the context go through `LoadBalancedAddress`.

Modifications:

- Add `LoadBalancedAddress#newConnection(ContextMap)` method;
- Deprecate `LoadBalancedAddress#newConnection()`;

Result:

Caller context can be propagated through `LoadBalancedAddress`.
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request Jul 22, 2022
Motivation:

apple#2168 add `ContextMap` for `LoadBalancer` and `ConnectionFactory`, but
does not let the context go through `LoadBalancedAddress`.

Modifications:

- Add `LoadBalancedAddress#newConnection(ContextMap)` method;
- Deprecate `LoadBalancedAddress#newConnection()`;

Result:

Caller context can be propagated through `LoadBalancedAddress`.
idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this pull request Jul 22, 2022
Motivation:

apple#2168 and apple#2235 added default implementations that throw an exception.
However, those method signatures return an async source.

Modifications:

- Return a `failed` source with an exception instead of throwing;

Result:

Default methods follow async contract.
idelpivnitskiy added a commit that referenced this pull request Jul 22, 2022
Motivation:

#2168 and #2235 added default implementations that throw an exception.
However, those method signatures return an async source.

Modifications:

- Return a `failed` source with an exception instead of throwing;

Result:

Default methods follow async contract.
idelpivnitskiy added a commit that referenced this pull request Jul 26, 2022
Motivation:

#2168 add `ContextMap` for `LoadBalancer` and `ConnectionFactory`, but
does not let the context go through `LoadBalancedAddress`.

Modifications:

- Add `LoadBalancedAddress#newConnection(ContextMap)` method;
- Deprecate `LoadBalancedAddress#newConnection()`;

Result:

Caller context can be propagated through `LoadBalancedAddress`.
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.

2 participants