BQ storage write api - Mint pool#3272
Conversation
ff9c25f to
6a3f159
Compare
Upgrades grpc ~> 0.11.0 and adds a persistent, supervised round-robin channel pool using the built-in Mint adapter to replace the per-call GRPC.Stub.connect (backed by the custom Finch adapter). - Add GrpcPool supervisor with per-index GrpcChannelMonitor GenServers; channels are registered in a per-pool Registry and selected via an atomic counter for lock-free round-robin dispatch - Add GrpcAuthInterceptor: fetches a fresh Goth token per call and injects it as gRPC metadata, replacing the per-call auth header on the channel - Remove local Google.Rpc.* protobuf definitions now provided by the googleapis transitive dep (pulled in by grpc 0.11.x) - Add GRPC.Client.Supervisor to Networking.pools/0 (required by grpc 0.11.x for outbound connections) - google_api_client: use GrpcPool.get_channel/1 and remove Goth.fetch; auth is now handled transparently by the interceptor
00d0357 to
1d15a33
Compare
| setup do | ||
| insert(:plan) | ||
| start_supervised!(AllLogsLogged) | ||
| start_supervised!(GRPC.Client.Supervisor) |
There was a problem hiding this comment.
Seems like unnecessary change as there is no client interaction in this test
There was a problem hiding this comment.
Actually, there is - in line 35, there's Client.connect that sends a request via localhost.
The change was needed due to :grpc version bump. The supervisor used to be started with :grpc app, but in the current version, it has to be started manually and isn't started in tests
| setup do | ||
| insert(:plan) | ||
| start_supervised!(AllLogsLogged) | ||
| start_supervised!(GRPC.Client.Supervisor) |
| insert(:plan) | ||
| start_supervised!(AllLogsLogged) | ||
| start_supervised!(GRPC.Client.Supervisor) | ||
|
|
| health: logflare_health, | ||
| http_connection_pools: http_connection_pools | ||
| http_connection_pools: http_connection_pools, | ||
| bq_write_api_pool_size: System.get_env("LOGFLARE_BQ_WRITE_API_POOL_SIZE") |
There was a problem hiding this comment.
| bq_write_api_pool_size: System.get_env("LOGFLARE_BQ_WRITE_API_POOL_SIZE") | |
| bq_write_api_pool_size: System.get_env("LOGFLARE_BIGQUERY_WRITE_API_POOL_SIZE") |
In my opinion we should automatically manage connection pool. But we can give this a try first. Needs a sensible default too.
Ziinc
left a comment
There was a problem hiding this comment.
Will merge in first for testing. Please address comments in follow up PRs 🙏
| {Logflare.Networking.GrpcPool, | ||
| name: GoogleApiClient.connetion_pool_name(), | ||
| url: "https://bigquerystorage.googleapis.com", | ||
| size: Application.get_env(:logflare, :bq_write_api_pool_size, 25)} |
There was a problem hiding this comment.
Default should be set at runtime.exs instead
Initial implementation of the Mint-adapter-based Storage Write API.
Retries and error handling will be covered by following PRs to keep this manageable.
Closes O11Y-1513