Skip to content

Commit 841b616

Browse files
svenefftingesvenefftinge
authored
[spicedb] fix stale connection handling (#18631)
1 parent 0e7f285 commit 841b616

File tree

17 files changed

+132
-108
lines changed

17 files changed

+132
-108
lines changed

‎components/content-service-api/typescript/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"lib"
1212
],
1313
"dependencies": {
14-
"@grpc/grpc-js": "1.8.8",
14+
"@grpc/grpc-js": "1.9.1",
1515
"google-protobuf": "^3.19.1",
1616
"inversify": "^6.0.1",
1717
"opentracing": "^0.14.4"

‎components/gitpod-protocol/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"src"
1111
],
1212
"devDependencies": {
13-
"@grpc/grpc-js": "1.8.8",
13+
"@grpc/grpc-js": "1.9.1",
1414
"@testdeck/mocha": "^0.3.3",
1515
"@types/analytics-node": "^3.1.9",
1616
"@types/chai-subset": "^1.3.3",

‎components/ide-metrics-api/typescript-grpc/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"lib"
1010
],
1111
"dependencies": {
12-
"@grpc/grpc-js": "1.8.8",
12+
"@grpc/grpc-js": "1.9.1",
1313
"google-protobuf": "^3.19.1"
1414
},
1515
"devDependencies": {

‎components/image-builder-api/typescript/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"dependencies": {
1414
"@gitpod/content-service": "0.1.5",
1515
"@gitpod/gitpod-protocol": "0.1.5",
16-
"@grpc/grpc-js": "1.8.8",
16+
"@grpc/grpc-js": "1.9.1",
1717
"google-protobuf": "^3.19.1",
1818
"inversify": "^6.0.1",
1919
"opentracing": "^0.14.4"

‎components/server/debug.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ kubectl scale deployment $deploymentName --replicas=1
2626
echo "Wait for the pod to be ready"
2727
kubectl wait --for=condition=ready pod -l component=$deploymentName
2828

29+
while [[ $(kubectl get pods -l component=$deploymentName -o 'jsonpath={..status.phase}' | grep -o 'Running' | wc -w) -ne 1 ]]; do
30+
echo "Waiting for scale down operation to complete..."
31+
sleep 1
32+
done
33+
2934
podName=$(kubectl get pods -l component=$deploymentName -o=jsonpath='{.items[0].metadata.name}')
3035
echo "Forward $podName port $debugPort to localhost. Waiting for a debugger to attach ..."
3136
kubectl port-forward pod/"$podName" $debugPort

‎components/server/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,5 +136,8 @@
136136
"ts-node": "^10.4.0",
137137
"typescript": "~4.4.2",
138138
"watch": "^1.0.2"
139+
},
140+
"resolutions": {
141+
"@authzed/authzed-node/**/@grpc/grpc-js": "1.9.1"
139142
}
140143
}

‎components/server/src/authorization/spicedb-authorizer.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ export class SpiceDBAuthorizer {
2121
private readonly clientProvider: SpiceDBClientProvider,
2222
) {}
2323

24+
private get client(): v1.ZedPromiseClientInterface {
25+
return this.clientProvider.getClient();
26+
}
27+
2428
async check(
2529
req: v1.CheckPermissionRequest,
2630
experimentsFields: {
@@ -39,8 +43,7 @@ export class SpiceDBAuthorizer {
3943
const timer = spicedbClientLatency.startTimer();
4044
let error: Error | undefined;
4145
try {
42-
const client = this.clientProvider.getClient();
43-
const response = await client.checkPermission(req, this.callOptions);
46+
const response = await this.client.checkPermission(req, this.callOptions);
4447
const permitted = response.permissionship === v1.CheckPermissionResponse_Permissionship.HAS_PERMISSION;
4548

4649
return permitted;
@@ -59,8 +62,7 @@ export class SpiceDBAuthorizer {
5962
const timer = spicedbClientLatency.startTimer();
6063
let error: Error | undefined;
6164
try {
62-
const client = this.clientProvider.getClient();
63-
const response = await client.writeRelationships(
65+
const response = await this.client.writeRelationships(
6466
v1.WriteRelationshipsRequest.create({
6567
updates,
6668
}),
@@ -81,11 +83,16 @@ export class SpiceDBAuthorizer {
8183
const timer = spicedbClientLatency.startTimer();
8284
let error: Error | undefined;
8385
try {
84-
const client = this.clientProvider.getClient();
85-
const existing = await client.readRelationships(v1.ReadRelationshipsRequest.create(req), this.callOptions);
86+
const existing = await this.client.readRelationships(
87+
v1.ReadRelationshipsRequest.create(req),
88+
this.callOptions,
89+
);
8690
if (existing.length > 0) {
87-
const response = await client.deleteRelationships(req, this.callOptions);
88-
const after = await client.readRelationships(v1.ReadRelationshipsRequest.create(req), this.callOptions);
91+
const response = await this.client.deleteRelationships(req, this.callOptions);
92+
const after = await this.client.readRelationships(
93+
v1.ReadRelationshipsRequest.create(req),
94+
this.callOptions,
95+
);
8996
if (after.length > 0) {
9097
log.error("[spicedb] Failed to delete relationships.", { existing, after, request: req });
9198
}
@@ -108,11 +115,7 @@ export class SpiceDBAuthorizer {
108115
}
109116

110117
async readRelationships(req: v1.ReadRelationshipsRequest): Promise<v1.ReadRelationshipsResponse[]> {
111-
const client = this.clientProvider.getClient();
112-
if (!client) {
113-
return [];
114-
}
115-
return client.readRelationships(req, this.callOptions);
118+
return this.client.readRelationships(req, this.callOptions);
116119
}
117120

118121
/**

‎components/server/src/authorization/spicedb.ts

Lines changed: 36 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,16 @@
55
*/
66

77
import { v1 } from "@authzed/authzed-node";
8-
import { IClientCallMetrics, createClientCallMetricsInterceptor } from "@gitpod/gitpod-protocol/lib/util/grpc";
98
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
109
import * as grpc from "@grpc/grpc-js";
1110

12-
export const SpiceDBClientProvider = Symbol("SpiceDBClientProvider");
13-
1411
export interface SpiceDBClientConfig {
1512
address: string;
1613
token: string;
1714
}
1815

1916
export type SpiceDBClient = v1.ZedPromiseClientInterface;
2017
type Client = v1.ZedClientInterface & grpc.Client;
21-
export interface SpiceDBClientProvider {
22-
getClient(): SpiceDBClient;
23-
}
2418

2519
export function spiceDBConfigFromEnv(): SpiceDBClientConfig | undefined {
2620
const token = process.env["SPICEDB_PRESHARED_KEY"];
@@ -40,63 +34,48 @@ export function spiceDBConfigFromEnv(): SpiceDBClientConfig | undefined {
4034
};
4135
}
4236

43-
function spicedbClientFromConfig(config: SpiceDBClientConfig): Client {
44-
const clientOptions: grpc.ClientOptions = {
45-
"grpc.client_idle_timeout_ms": 10000, // this ensures a connection is not stuck in the "READY" state for too long. Required for the reconnect logic below.
46-
"grpc.max_reconnect_backoff_ms": 5000, // default: 12000
47-
};
48-
49-
return v1.NewClient(
50-
config.token,
51-
config.address,
52-
v1.ClientSecurity.INSECURE_PLAINTEXT_CREDENTIALS,
53-
undefined,
54-
clientOptions,
55-
) as Client;
56-
}
57-
58-
export class CachingSpiceDBClientProvider implements SpiceDBClientProvider {
37+
export class SpiceDBClientProvider {
5938
private client: Client | undefined;
6039

61-
private readonly interceptors: grpc.Interceptor[] = [];
62-
6340
constructor(
64-
private readonly _clientConfig: SpiceDBClientConfig,
65-
private readonly clientCallMetrics?: IClientCallMetrics | undefined,
66-
) {
67-
if (this.clientCallMetrics) {
68-
this.interceptors.push(createClientCallMetricsInterceptor(this.clientCallMetrics));
69-
}
70-
}
41+
private readonly clientConfig: SpiceDBClientConfig,
42+
private readonly interceptors: grpc.Interceptor[] = [],
43+
) {}
7144

7245
getClient(): SpiceDBClient {
73-
let client = this.client;
74-
if (!client) {
75-
client = spicedbClientFromConfig(this.clientConfig);
76-
} else if (client.getChannel().getConnectivityState(true) !== grpc.connectivityState.READY) {
77-
// (gpl): We need this check and explicit re-connect because we observe a ~120s connection timeout without it.
78-
// It's not entirely clear where that timeout comes from, but likely from the underlying transport, that is not exposed by grpc/grpc-js
79-
client.close();
80-
81-
log.warn("[spicedb] Lost connection to SpiceDB - reconnecting...");
82-
83-
client = spicedbClientFromConfig(this.clientConfig);
84-
}
85-
this.client = client;
86-
87-
return client.promises;
88-
}
89-
90-
protected get clientConfig() {
91-
const config = this._clientConfig;
92-
if (this.interceptors) {
93-
return {
94-
...config,
95-
options: {
96-
interceptors: [...this.interceptors],
46+
if (!this.client) {
47+
this.client = v1.NewClient(
48+
this.clientConfig.token,
49+
this.clientConfig.address,
50+
v1.ClientSecurity.INSECURE_PLAINTEXT_CREDENTIALS,
51+
undefined, //
52+
{
53+
// we ping frequently to check if the connection is still alive
54+
"grpc.keepalive_time_ms": 1000,
55+
"grpc.keepalive_timeout_ms": 1000,
56+
57+
"grpc.max_reconnect_backoff_ms": 5000,
58+
"grpc.initial_reconnect_backoff_ms": 500,
59+
"grpc.service_config": JSON.stringify({
60+
methodConfig: [
61+
{
62+
name: [{}],
63+
retryPolicy: {
64+
maxAttempts: 10,
65+
initialBackoff: "0.1s",
66+
maxBackoff: "5s",
67+
backoffMultiplier: 2.0,
68+
retryableStatusCodes: ["UNAVAILABLE", "DEADLINE_EXCEEDED"],
69+
},
70+
},
71+
],
72+
}),
73+
74+
"grpc.enable_retries": 1, //TODO enabled by default
75+
interceptors: this.interceptors,
9776
},
98-
};
77+
) as Client;
9978
}
100-
return config;
79+
return this.client.promises;
10180
}
10281
}

‎components/server/src/container-module.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import { GitpodFileParser } from "@gitpod/gitpod-protocol/lib/gitpod-file-parser
1111
import { PrometheusClientCallMetrics } from "@gitpod/gitpod-protocol/lib/messaging/client-call-metrics";
1212
import { newAnalyticsWriterFromEnv } from "@gitpod/gitpod-protocol/lib/util/analytics";
1313
import { DebugApp } from "@gitpod/gitpod-protocol/lib/util/debug-app";
14-
import { IClientCallMetrics, defaultGRPCOptions } from "@gitpod/gitpod-protocol/lib/util/grpc";
14+
import {
15+
IClientCallMetrics,
16+
createClientCallMetricsInterceptor,
17+
defaultGRPCOptions,
18+
} from "@gitpod/gitpod-protocol/lib/util/grpc";
1519
import { prometheusClientMiddleware } from "@gitpod/gitpod-protocol/lib/util/nice-grpc";
1620
import { IDEServiceClient, IDEServiceDefinition } from "@gitpod/ide-service-api/lib/ide.pb";
1721
import { ImageBuilderClientCallMetrics, ImageBuilderClientProvider } from "@gitpod/image-builder/lib";
@@ -46,7 +50,7 @@ import { AuthJWT, SignInJWT } from "./auth/jwt";
4650
import { LoginCompletionHandler } from "./auth/login-completion-handler";
4751
import { VerificationService } from "./auth/verification-service";
4852
import { Authorizer, createInitializingAuthorizer } from "./authorization/authorizer";
49-
import { CachingSpiceDBClientProvider, SpiceDBClientProvider, spiceDBConfigFromEnv } from "./authorization/spicedb";
53+
import { SpiceDBClientProvider, spiceDBConfigFromEnv } from "./authorization/spicedb";
5054
import { BillingModes } from "./billing/billing-mode";
5155
import { EntitlementService, EntitlementServiceImpl } from "./billing/entitlement-service";
5256
import { EntitlementServiceUBP } from "./billing/entitlement-service-ubp";
@@ -309,7 +313,10 @@ export const productionContainerModule = new ContainerModule(
309313
throw new Error("[spicedb] Missing configuration expected in env vars!");
310314
}
311315
const clientCallMetrics = ctx.container.get<IClientCallMetrics>(IClientCallMetrics);
312-
return new CachingSpiceDBClientProvider(config, clientCallMetrics);
316+
return new SpiceDBClientProvider(
317+
config, //
318+
[createClientCallMetricsInterceptor(clientCallMetrics)],
319+
);
313320
})
314321
.inSingletonScope();
315322
bind(SpiceDBAuthorizer).toSelf().inSingletonScope();

‎components/server/src/test/service-testing-container-module.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { v4 } from "uuid";
1313
import { AuthProviderParams } from "../auth/auth-provider";
1414
import { HostContextProvider, HostContextProviderFactory } from "../auth/host-context-provider";
1515
import { HostContextProviderImpl } from "../auth/host-context-provider-impl";
16-
import { CachingSpiceDBClientProvider, SpiceDBClientProvider } from "../authorization/spicedb";
16+
import { SpiceDBClientProvider } from "../authorization/spicedb";
1717
import { Config } from "../config";
1818
import { StorageClient } from "../storage/storage-client";
1919
import { testContainer } from "@gitpod/gitpod-db/lib";
@@ -194,7 +194,7 @@ const mockApplyingContainerModule = new ContainerModule((bind, unbound, isbound,
194194
token: v4(),
195195
address: "localhost:50051",
196196
};
197-
return new CachingSpiceDBClientProvider(config);
197+
return new SpiceDBClientProvider(config);
198198
})
199199
.inSingletonScope();
200200
});

‎components/supervisor-api/typescript-grpc/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"lib"
1010
],
1111
"dependencies": {
12-
"@grpc/grpc-js": "1.8.8",
12+
"@grpc/grpc-js": "1.9.1",
1313
"google-protobuf": "^3.19.1"
1414
},
1515
"devDependencies": {

‎components/usage-api/typescript/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"ts-proto": "^1.153.0"
1919
},
2020
"devDependencies": {
21+
"@types/long": "4.0.0",
2122
"grpc-tools": "^1.12.4",
2223
"typescript": "~4.4.2",
2324
"typescript-formatter": "^7.2.2"

‎components/ws-daemon-api/typescript/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
],
1414
"dependencies": {
1515
"@gitpod/content-service": "0.1.5",
16-
"@grpc/grpc-js": "1.8.8",
16+
"@grpc/grpc-js": "1.9.1",
1717
"google-protobuf": "^3.19.1",
1818
"inversify": "^6.0.1",
1919
"opentracing": "^0.14.4"

‎components/ws-manager-api/typescript/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"dependencies": {
1717
"@gitpod/content-service": "0.1.5",
1818
"@gitpod/gitpod-protocol": "0.1.5",
19-
"@grpc/grpc-js": "1.8.8",
19+
"@grpc/grpc-js": "1.9.1",
2020
"google-protobuf": "^3.19.1",
2121
"inversify": "^6.0.1",
2222
"opentracing": "^0.14.4"

‎components/ws-manager-bridge-api/typescript/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"lib"
1212
],
1313
"dependencies": {
14-
"@grpc/grpc-js": "1.8.8",
14+
"@grpc/grpc-js": "1.9.1",
1515
"google-protobuf": "^3.19.1",
1616
"inversify": "^6.0.1",
1717
"opentracing": "^0.14.4"

‎install/installer/pkg/components/spicedb/deployment.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
154154
Command: []string{"grpc_health_probe", "-v", fmt.Sprintf("-addr=localhost:%d", ContainerGRPCPort)},
155155
},
156156
},
157-
InitialDelaySeconds: 5,
158-
// try again every 10 seconds
159-
PeriodSeconds: 10,
160-
// fail after 6 * 10 + 5 = 65 seconds
161-
FailureThreshold: 6,
157+
InitialDelaySeconds: 1,
158+
// try again every 2 seconds
159+
PeriodSeconds: 2,
160+
// fail after 30 * 2 + 1 = 61
161+
FailureThreshold: 30,
162162
SuccessThreshold: 1,
163-
TimeoutSeconds: 3,
163+
TimeoutSeconds: 1,
164164
},
165165
VolumeMounts: []v1.VolumeMount{
166166
bootstrapVolumeMount,

0 commit comments

Comments
 (0)