Skip to content

Commit

Permalink
Merge #66317 #66428
Browse files Browse the repository at this point in the history
66317: sqlproxyccl: remove unused denylist implementations r=JeffSwenson a=JeffSwenson

The viper and mock denylist formats are no longer used as of #66137. I'm refactoring 
the denylist for issue #65694. Removing the unused format simplifies the refactoring.

Release note: None

66428: server: TestTelemetrySQLStatsIndependence uses test reg server r=knz,nkodali a=dhartunian

Previously, this test hit the real registration server which caused
goroutine leaks when the reg server was unavailable.

The test now creates a test reg server to handle the stats calls.
The test is no longer skipped.

Resolves #63851

Release note: None

Co-authored-by: Jeff Swenson <[email protected]>
Co-authored-by: David Hartunian <[email protected]>
  • Loading branch information
3 people committed Jun 14, 2021
3 parents 25a1527 + fa77232 + 871b8e5 commit 9c583b9
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 224 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ require (
github.com/shopspring/decimal v1.2.0 // indirect
github.com/spf13/cobra v1.1.3
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.7.1
github.com/stretchr/testify v1.6.1
github.com/twpayne/go-geom v1.3.7-0.20210228220813-9d9885b50d3e
github.com/wadey/gocovmerge v0.0.0-20160331181800-b5bfa59ec0ad
Expand Down
24 changes: 2 additions & 22 deletions pkg/ccl/sqlproxyccl/denylist/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
load("@bazel_gomock//:gomock.bzl", "gomock")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

# gazelle:exclude service.go
Expand All @@ -12,32 +11,22 @@ go_library(

go_library(
name = "denylist",
srcs = [
"file.go",
"local_file.go",
":mocks_denylist", # keep
],
srcs = ["file.go"],
embed = [":service"], # keep
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/sqlproxyccl/denylist",
visibility = ["//visibility:public"],
deps = [
"//pkg/util",
"//pkg/util/log",
"//pkg/util/syncutil",
"//pkg/util/timeutil",
"@com_github_cockroachdb_errors//:errors",
"@com_github_golang_mock//gomock", # keep
"@com_github_spf13_viper//:viper",
"@in_gopkg_yaml_v2//:yaml_v2",
],
)

go_test(
name = "denylist_test",
srcs = [
"file_test.go",
"local_file_test.go",
],
srcs = ["file_test.go"],
embed = [":denylist"],
deps = [
"//pkg/util/leaktest",
Expand All @@ -46,12 +35,3 @@ go_test(
"@in_gopkg_yaml_v2//:yaml_v2",
],
)

gomock(
name = "mocks_denylist",
out = "mocks_generated.go",
interfaces = ["Service"],
library = ":service",
package = "denylist",
self_package = "github.com/cockroachdb/cockroach/pkg/ccl/sqlproxyccl/denylist",
)
104 changes: 0 additions & 104 deletions pkg/ccl/sqlproxyccl/denylist/local_file.go

This file was deleted.

90 changes: 0 additions & 90 deletions pkg/ccl/sqlproxyccl/denylist/local_file_test.go

This file was deleted.

1 change: 1 addition & 0 deletions pkg/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ go_test(
"//pkg/storage",
"//pkg/storage/enginepb",
"//pkg/testutils",
"//pkg/testutils/diagutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
Expand Down
18 changes: 12 additions & 6 deletions pkg/server/stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sqlstats"
"github.com/cockroachdb/cockroach/pkg/sql/tests"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/diagutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand All @@ -39,13 +39,19 @@ func TestTelemetrySQLStatsIndependence(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

// This test fails if the central reporting server from CRL
// breaks. It should be improved to customize the reporting URL and
// mock the registration collector with an in-memory server.
skip.WithIssue(t, 63851)

ctx := context.Background()
params, _ := tests.CreateTestServerParams()

r := diagutils.NewServer()
defer r.Close()

url := r.URL()
params.Knobs.Server = &TestingKnobs{
DiagnosticsTestingKnobs: diagnostics.TestingKnobs{
OverrideReportingURL: &url,
},
}

s, sqlDB, _ := serverutils.StartServer(t, params)
defer s.Stopper().Stop(ctx)

Expand Down
2 changes: 1 addition & 1 deletion vendor
Submodule vendor updated 1 files
+0 −1 modules.txt

0 comments on commit 9c583b9

Please sign in to comment.