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

control-plane proxies connector invocations through data-planes #1601

Merged
merged 7 commits into from
Sep 5, 2024

Conversation

jgraettinger
Copy link
Member

@jgraettinger jgraettinger commented Sep 2, 2024

Description:

  • Introduce a ConnectorsProxy gRPC service and implement it within the reactor.
  • Update the control-plane to delegate all Validate and Discover RPCs to designated data-planes, using the new service.

See individual commits for much more detail.

Fixes #1602

Workflow steps:

No changes to UX. Note that RPCs are now started against reactors as-needed to satisfy control-plane connector invocations.

Documentation links affected:

(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)

Notes for reviewers:

(anything that might help someone review this PR)


This change is Reviewable

@jgraettinger jgraettinger force-pushed the johnny/proxy-connectors branch from c343a97 to 6240858 Compare September 2, 2024 17:00
@jgraettinger
Copy link
Member Author

Testing

Last-resort connector timeouts still work:

image

Formatted connector errors still work:

image

We get streaming logs at expected log levels:

image

@jgraettinger jgraettinger force-pushed the johnny/proxy-connectors branch from 6240858 to ac9f6cb Compare September 2, 2024 22:46
@jgraettinger jgraettinger marked this pull request as ready for review September 2, 2024 22:46
@jgraettinger jgraettinger requested a review from psFried September 2, 2024 22:46
@jgraettinger jgraettinger added the change:planned This is a planned change label Sep 3, 2024
Copy link
Member

@psFried psFried left a comment

Choose a reason for hiding this comment

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

LGTM
I left a few questions/suggestions, but none are necessarily blocking.

The ConnectorProxy service starts an identified, ephemeral runtime
which exists for the duration of the RPC.

It can then be referenced by other RPCs dispatched to the designated
address.

Issue #1602
Where before it took an ops.Publisher, which is used to attach a ShardRef
to the log, it now takes a `func(ops.Log)`.

Instead, OpsPublisher is updated to attach the *ShardRef it already has
to the Log prior to writting it out to an ops journal. Basically, we're
just moving the place where this happens down the stack.

This allows LogWriteAdapter to be used with log handlers which _don't_
want to have this behavior, or don't know the proper *ShardRef.

Issue #1602
@jgraettinger jgraettinger force-pushed the johnny/proxy-connectors branch from ac9f6cb to 19e43cc Compare September 5, 2024 15:12
`connectorProxy` implements ProxyConnectors by starting a TaskService
with a lifetime bound to the RPC, and with a semaphore to constrain
maximum concurrency.

Logs are streamed back to the caller as they're produced, and a
`proxy-id` is returned which is then expected to be attached to various
connector RPCs.

Issue #1602
gazette::Metadata is metadata attached to a gRPC request, which is
inclusive of authorization and other headers.

Add new routines for signing Gazette-centric authorization tokens.

Also refactor out and expose low-level routines for dialing channels.
NoOpWrapper encapsulates a common pattern of dynamically disabling
validations for various connector types.

Remove timeouts from unary RPCs directly invoked against the runtime.
These timeouts are moving into the `agent`, which is the only place we
actually care to time out these requests. When running locally with
`flowctl`, for example, we never want to time out.
ProxyConnectors are used for all Validate RPCs and Discover.

It's not (yet) used for connector spec requests.

Issue #1602
@jgraettinger jgraettinger force-pushed the johnny/proxy-connectors branch from 19e43cc to 9273008 Compare September 5, 2024 16:32
@jgraettinger
Copy link
Member Author

Completed another round of self-review and local testing, and caught a couple of things (now fixed).

@jgraettinger jgraettinger merged commit 1234e04 into master Sep 5, 2024
5 checks passed
@jgraettinger jgraettinger deleted the johnny/proxy-connectors branch September 5, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:planned This is a planned change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

control-plane proxies connector RPCs through data-planes
2 participants