-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
105316: obsservice: migrate gRPC ingest to follow new architecture r=knz a=abarganier **Reviewer note: review this PR commit-wise.** ---- In the original design of the obsservice, exported events were intended to be written directly to storage. The idea was that exported events would experience minimal transformation once ingested, meaning that work done to "package" events properly was left up to the exporting client (CRDB). The obsservice would then store the ingested invents into a target storage. This concept of target storage has been removed for now as part of this patch. In the new architecture, exported events are more "raw", and we expect the obsservice to heavily transform & aggregate the data externally, where the aggregated results are flushed to storage instead. This patch takes the pre-existing gRPC events ingester, and modifies it to meet the new architecture. The events ingester will now be provided with a consumer with which it can feed ingested events into the broader pipeline. It is no longer the responsibility of the ingester to write ingested events to storage. For now, we use a simple STDOUT consumer that writes all ingested events to STDOUT, but in the future, this will be a more legitimate component - part of a chain that eventually buffers ingested events for aggregation. Release note: none Epic: CRDB-28526 105589: ccl/sqlproxyccl: fix possible flake in TestProxyProtocol r=pjtatlow a=jaylim-crl Fixes #105585. This commit updates the TestProxyProtocol test to only test the case where RequireProxyProtocol=true. There's no point testing the case where the RequireProxyProtocol field is false since every other tests do not use the proxy protocol (and that case is implicitly covered by them). It's unclear what is causing this test flake (and it is extremely rare, i.e. 1 legit failure out of 1000 runs [1]). It may be due to some sort of race within the tests, but given that the case is covered by all other tests, this commit opts to remove the test entirely. [1] https://teamcity.cockroachdb.com/test/-1121006080109385641?currentProjectId=Cockroach_Ci_TestsGcpLinuxX8664BigVm&expandTestHistoryChartSection=true Release note: None Release justification: Fixes a test flake. Epic: none 105630: roachtest: handle panics in `mixedversion` r=smg260 a=renatolabs Previously, a panic in a user function in a roachtest using the `mixedversion` package would crash the entire roachtest process. This is because all steps run in a separate goroutine, so if panics are not captured, the entire process crashes. This commit updates the test runner so that all steps (including those that are part of the test infrastructure) run with panics captured. The panic message is returned as a regular error which should lead to usual GitHub error reports. The stack trace for the panic is also logged so that we can pinpoint the exact offending line in the test. Epic: CRDB-19321 Release note: None Co-authored-by: Alex Barganier <[email protected]> Co-authored-by: Jay <[email protected]> Co-authored-by: Renato Costa <[email protected]>
- Loading branch information
Showing
34 changed files
with
1,076 additions
and
519 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
85 changes: 85 additions & 0 deletions
85
pkg/cmd/roachtest/roachtestutil/mixedversion/runner_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| // Copyright 2023 The Cockroach Authors. | ||
| // | ||
| // Use of this software is governed by the Business Source License | ||
| // included in the file licenses/BSL.txt. | ||
| // | ||
| // As of the Change Date specified in that file, in accordance with | ||
| // the Business Source License, use of this software will be governed | ||
| // by the Apache License, Version 2.0, included in the file | ||
| // licenses/APL.txt. | ||
|
|
||
| package mixedversion | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" | ||
| "github.com/cockroachdb/cockroach/pkg/roachprod/logger" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func Test_runSingleStep(t *testing.T) { | ||
| tr := testTestRunner() | ||
|
|
||
| // steps that run without errors do not return errors | ||
| successStep := newTestStep(func() error { | ||
| return nil | ||
| }) | ||
| err := tr.runSingleStep(ctx, successStep, nilLogger) | ||
| require.NoError(t, err) | ||
|
|
||
| // steps that return an error have that error surfaced | ||
| errorStep := newTestStep(func() error { | ||
| return fmt.Errorf("oops") | ||
| }) | ||
| err = tr.runSingleStep(ctx, errorStep, nilLogger) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "oops") | ||
|
|
||
| // steps that panic cause an error to be returned | ||
| panicStep := newTestStep(func() error { | ||
| var ids []int | ||
| if ids[0] > 42 { | ||
| return nil | ||
| } | ||
| return fmt.Errorf("unreachable") | ||
| }) | ||
| err = nil | ||
| require.NotPanics(t, func() { | ||
| err = tr.runSingleStep(ctx, panicStep, nilLogger) | ||
| }) | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "panic (stack trace above): runtime error: index out of range [0] with length 0") | ||
| } | ||
|
|
||
| func testTestRunner() *testRunner { | ||
| runnerCtx, cancel := context.WithCancel(ctx) | ||
| return &testRunner{ | ||
| ctx: runnerCtx, | ||
| cancel: cancel, | ||
| logger: nilLogger, | ||
| crdbNodes: nodes, | ||
| background: newBackgroundRunner(runnerCtx), | ||
| seed: seed, | ||
| } | ||
| } | ||
|
|
||
| type testSingleStep struct { | ||
| runFunc func() error | ||
| } | ||
|
|
||
| func (testSingleStep) ID() int { return 42 } | ||
| func (testSingleStep) Description() string { return "testSingleStep" } | ||
| func (testSingleStep) Background() shouldStop { return nil } | ||
|
|
||
| func (tss testSingleStep) Run( | ||
| _ context.Context, _ *logger.Logger, _ cluster.Cluster, _ *Helper, | ||
| ) error { | ||
| return tss.runFunc() | ||
| } | ||
|
|
||
| func newTestStep(f func() error) singleStep { | ||
| return testSingleStep{runFunc: f} | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.