diff --git a/.github/workflows/go-cover.yml b/.github/workflows/go-cover.yml index 5af385e99b6..7a7f3eb0c1a 100644 --- a/.github/workflows/go-cover.yml +++ b/.github/workflows/go-cover.yml @@ -11,18 +11,12 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/setup-go@v5 + - name: Check out code + uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 with: go-version: 1.23 - - uses: actions/checkout@v4 - - uses: actions/cache@v4 - with: - path: | - ~/go/pkg/mod - ~/.cache/go-build - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- - name: Go test with coverage run: |- # Build list of packages to include in coverage, excluding generated code in 'proto/gen' diff --git a/.github/workflows/go-lint.yml b/.github/workflows/go-lint.yml index 5dd94bda2dc..7e54c0b07e1 100644 --- a/.github/workflows/go-lint.yml +++ b/.github/workflows/go-lint.yml @@ -14,7 +14,8 @@ jobs: name: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - name: Check out code + uses: actions/checkout@v4 - name: Set up Go uses: actions/setup-go@v5 with: diff --git a/.github/workflows/go-test.yml b/.github/workflows/go-test.yml index 7f21e7a198c..6b9a30d2e57 100644 --- a/.github/workflows/go-test.yml +++ b/.github/workflows/go-test.yml @@ -10,18 +10,12 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/setup-go@v4 + - name: Check out code + uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 with: go-version: 1.23 - - uses: actions/checkout@v4 - - uses: actions/cache@v4 - with: - path: | - ~/go/pkg/mod - ~/.cache/go-build - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- - name: Go fmt run: test -z $(gofmt -l .) - name: Go test diff --git a/.github/workflows/rill-cloud.yml b/.github/workflows/rill-cloud.yml index 95b3ac2f5e2..062b47ee153 100644 --- a/.github/workflows/rill-cloud.yml +++ b/.github/workflows/rill-cloud.yml @@ -28,7 +28,7 @@ jobs: fetch-tags: true - name: Set up Go - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: 1.23 diff --git a/.github/workflows/web-test-e2e.yml b/.github/workflows/web-test-e2e.yml index bc26b5d4b36..87da4f2787d 100644 --- a/.github/workflows/web-test-e2e.yml +++ b/.github/workflows/web-test-e2e.yml @@ -96,16 +96,6 @@ jobs: with: go-version: 1.23 - - name: go build cache - uses: actions/cache@v4 - with: - path: | - ~/go/pkg/mod - ~/.cache/go-build - key: ${{ runner.os }}-go-${{ hashFiles('./go.sum') }} - restore-keys: | - ${{ runner.os }}-go- - - name: Build and embed static UI if: matrix.name == 'web-local' run: PLAYWRIGHT_TEST=true make cli diff --git a/admin/server/rbac_test.go b/admin/server/rbac_test.go index 1108a9e3260..b6edaf6d2b2 100644 --- a/admin/server/rbac_test.go +++ b/admin/server/rbac_test.go @@ -819,6 +819,7 @@ func TestRBAC(t *testing.T) { Domain: "whitelist-projs.test", Role: database.ProjectRoleNameAdmin, }) + require.NoError(t, err) // Check we can't whitelist the same domain on the same project again _, err = c1.CreateProjectWhitelistedDomain(ctx, &adminv1.CreateProjectWhitelistedDomainRequest{ diff --git a/admin/server/util_test.go b/admin/server/util_test.go index fb6ecaa8f96..947a6607e63 100644 --- a/admin/server/util_test.go +++ b/admin/server/util_test.go @@ -84,7 +84,6 @@ func newTestServer(t *testing.T) *Server { // Sender sender := email.NewTestSender() - require.NoError(t, err) emailClient := email.New(sender) // Application-managed column encryption keyring diff --git a/runtime/pkg/graceful/grpc.go b/runtime/pkg/graceful/grpc.go index 0976803388d..295846230b7 100644 --- a/runtime/pkg/graceful/grpc.go +++ b/runtime/pkg/graceful/grpc.go @@ -2,13 +2,17 @@ package graceful import ( "context" + "errors" "fmt" "net" "strings" + "time" "google.golang.org/grpc" ) +const grpcShutdownTimeout = 15 * time.Second + // ServeGRPC serves a GRPC server and performs a graceful shutdown if/when ctx is cancelled. func ServeGRPC(ctx context.Context, server *grpc.Server, port int) error { // Calling net.Listen("tcp", ...) will succeed if the port is blocked on IPv4 but not on IPv6. @@ -25,17 +29,27 @@ func ServeGRPC(ctx context.Context, server *grpc.Server, port int) error { return err } - cctx, cancel := context.WithCancel(ctx) - var serveErr error + // Channel to signal server has stopped + serveErrCh := make(chan error) go func() { - serveErr = server.Serve(lis) - cancel() + err := server.Serve(lis) + serveErrCh <- err }() - <-cctx.Done() - if serveErr == nil { + // Wait for context to be cancelled or failure to serve + select { + case err := <-serveErrCh: + return err + case <-ctx.Done(): server.GracefulStop() } - return serveErr + // Wait for graceful shutdown + select { + case err := <-serveErrCh: + return err + case <-time.After(grpcShutdownTimeout): + server.Stop() + return errors.New("grpc graceful shutdown timed out") + } } diff --git a/runtime/pkg/graceful/http.go b/runtime/pkg/graceful/http.go index 79712a2e4be..9d4ca58c3f5 100644 --- a/runtime/pkg/graceful/http.go +++ b/runtime/pkg/graceful/http.go @@ -33,26 +33,29 @@ func ServeHTTP(ctx context.Context, server *http.Server, options ServeOptions) e return err } - cctx, cancel := context.WithCancel(ctx) - var serveErr error + // Channel to signal server has stopped + serveErrCh := make(chan error) + // Start server in a goroutine go func() { if options.CertPath != "" && options.KeyPath != "" { - serveErr = server.ServeTLS(lis, options.CertPath, options.KeyPath) + // Use HTTPS if cert and key are provided + err := server.ServeTLS(lis, options.CertPath, options.KeyPath) + serveErrCh <- err } else { - serveErr = server.Serve(lis) + // Otherwise use HTTP + err := server.Serve(lis) + serveErrCh <- err } - - cancel() }() - <-cctx.Done() - if serveErr == nil { - // server.Serve always returns a non-nil err, so this must be a cancel on the parent ctx. - // We perform a graceful shutdown. + // Wait for context cancellation or server stopped + select { + case err := <-serveErrCh: + return err + case <-ctx.Done(): ctx, cancel := context.WithTimeout(context.Background(), httpShutdownTimeout) defer cancel() - serveErr = server.Shutdown(ctx) - } - return serveErr + return server.Shutdown(ctx) + } } diff --git a/runtime/runtime.go b/runtime/runtime.go index 58f92170e2c..bab4a6d6a31 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sync" "time" runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1" @@ -42,6 +43,7 @@ type Runtime struct { connCache conncache.Cache queryCache *queryCache securityEngine *securityEngine + instanceMu sync.RWMutex } func New(ctx context.Context, opts *Options, logger *zap.Logger, st *storage.Client, ac *activity.Client, emailClient *email.Client) (*Runtime, error) { diff --git a/runtime/server/queries_metrics_comparison_toplist_test.go b/runtime/server/queries_metrics_comparison_toplist_test.go index e116122a2a2..99122c62eb3 100644 --- a/runtime/server/queries_metrics_comparison_toplist_test.go +++ b/runtime/server/queries_metrics_comparison_toplist_test.go @@ -168,10 +168,10 @@ func TestServer_MetricsViewComparison_inline_measures(t *testing.T) { } func TestServer_MetricsViewComparison_nulls(t *testing.T) { + t.Parallel() // NOTE: Unstable due to sleep. Commenting until we support configuring settings at instance create time. t.Skip() - t.Parallel() server, instanceId := getMetricsTestServer(t, "ad_bids_2rows") // TODO: Support configuring at create time