From 1de2341937127a0c8d11f60fe9d48165691dafe8 Mon Sep 17 00:00:00 2001 From: Alex Barganier Date: Mon, 20 Mar 2023 15:56:45 -0400 Subject: [PATCH] pkg/ccl: Unskip TestTenantStatusAPI/tenant_ranges/pagination Fixes: https://github.com/cockroachdb/cockroach/issues/92979 Previously, in https://github.com/cockroachdb/cockroach/pull/97386, we skipped test_tenant_ranges_pagination because it was marked as flaky. The test makes a request for a single range and expects an offset of `1` back. It then uses this offset to request a second range, and expects an offset of `2`. This means that the test requires at least 3 ranges to exist on the tenant. The test was flaking on the assertion that the offset returned by the second request came back as `2`. Instead, it was flaking when the offset came back as `0`, which signifies that there are no more ranges to process. We learned that the tenant create process has an asycnhronous splitting of ranges that occurs, which is what would lead to this sporadic scenario where not enough ranges existed (yet) for the test to succeed. This patch updates the test with a `testutils.SucceedsSoon` clause that checks first that `crdb_internal.ranges` contains at least 3 ranges, prior to making the second request. This should provide sufficient time for the range split queue to be processed and eliminate the vast majority of these test flakes. Release note: none --- .../serverccl/statusccl/tenant_status_test.go | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/pkg/ccl/serverccl/statusccl/tenant_status_test.go b/pkg/ccl/serverccl/statusccl/tenant_status_test.go index 17aebfc3aa24..9d056c8b674c 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go @@ -1177,18 +1177,34 @@ func testTenantRangesRPC(_ context.Context, t *testing.T, helper serverccl.Tenan require.Len(t, ranges.Ranges, 1) } - resp2, err := tenantA.TenantRanges(ctx, &serverpb.TenantRangesRequest{ - Limit: 1, - Offset: resp1.Next, + sql := helper.TestCluster().TenantConn(0) + // Wait for the split queue to process some before requesting the 2nd range. + // We expect an offset for the 3rd range, so wait until at least 3 ranges exist. + testutils.SucceedsSoon(t, func() error { + res := sql.QueryStr(t, "SELECT count(*) FROM crdb_internal.ranges") + require.Equal(t, len(res), 1) + require.Equal(t, len(res[0]), 1) + rangeCount, err := strconv.Atoi(res[0][0]) + require.NoError(t, err) + if rangeCount < 3 { + return errors.Newf("expected >= 3 ranges, got %d", rangeCount) + } + + resp2, err := tenantA.TenantRanges(ctx, &serverpb.TenantRangesRequest{ + Limit: 1, + Offset: resp1.Next, + }) + require.NoError(t, err) + require.Equal(t, 2, int(resp2.Next)) + for locality, ranges := range resp2.RangesByLocality { + require.Len(t, ranges.Ranges, 1) + // Verify pagination functions based on ascending RangeID order. + require.True(t, + resp1.RangesByLocality[locality].Ranges[0].RangeID < ranges.Ranges[0].RangeID) + } + return nil }) - require.NoError(t, err) - require.Equal(t, 2, int(resp2.Next)) - for locality, ranges := range resp2.RangesByLocality { - require.Len(t, ranges.Ranges, 1) - // Verify pagination functions based on ascending RangeID order. - require.True(t, - resp1.RangesByLocality[locality].Ranges[0].RangeID < ranges.Ranges[0].RangeID) - } + }) }