Skip to content

Commit

Permalink
storage: restore range information in AdminScatterResponse
Browse files Browse the repository at this point in the history
`ALTER TABLE... SCATTER` expects to receive a list of ranges that were
scattered. This information was accidentally dropped in the new
scatter implementation (dbd90cf, cockroachdb#16249). This commit restores the old
behavior, and adds a test to boot.

Fixes cockroachdb#17153.
  • Loading branch information
benesch committed Aug 7, 2017
1 parent 0eaeff2 commit 59abcf4
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
55 changes: 55 additions & 0 deletions pkg/sql/split_at_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ import (
"golang.org/x/net/context"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand Down Expand Up @@ -194,3 +196,56 @@ func TestScatter(t *testing.T) {
}
}
}

// TestScatterResponse ensures that ALTER TABLE... SCATTER includes one row of
// output per range in the table. It does *not* test that scatter properly
// distributes replicas and leases; see TestScatter for that.
//
// TODO(benesch): consider folding this test into TestScatter once TestScatter
// is unskipped.
func TestScatterResponse(t *testing.T) {
defer leaktest.AfterTest(t)()

s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
defer s.Stopper().Stop(context.Background())

sqlutils.CreateTable(
t, sqlDB, "t",
"k INT PRIMARY KEY, v INT",
1000,
sqlutils.ToRowFn(sqlutils.RowIdxFn, sqlutils.RowModuloFn(10)),
)
tableDesc := sqlbase.GetTableDescriptor(kvDB, "test", "t")

r := sqlutils.MakeSQLRunner(t, sqlDB)
r.Exec("ALTER TABLE test.t SPLIT AT (SELECT i*10 FROM generate_series(1, 99) AS g(i))")
rows := r.Query("ALTER TABLE test.t SCATTER")

i := 0
for ; rows.Next(); i++ {
var actualKey []byte
var pretty string
if err := rows.Scan(&actualKey, &pretty); err != nil {
t.Fatal(err)
}
var expectedKey roachpb.Key
if i == 0 {
expectedKey = keys.MakeTablePrefix(uint32(tableDesc.ID))
} else {
var err error
expectedKey, err = sqlbase.MakePrimaryIndexKey(tableDesc, i*10)
if err != nil {
t.Fatal(err)
}
}
if e, a := expectedKey, roachpb.Key(actualKey); !e.Equal(a) {
t.Errorf("%d: expected split key %s, but got %s", i, e, a)
}
if e, a := expectedKey.String(), pretty; e != a {
t.Errorf("%d: expected pretty split key %s, but got %s", i, e, a)
}
}
if e, a := 100, i; e != a {
t.Fatalf("expected %d rows, but got %d", e, a)
}
}
9 changes: 8 additions & 1 deletion pkg/storage/replica_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -4070,5 +4070,12 @@ func (r *Replica) adminScatter(
return roachpb.AdminScatterResponse{}, ctx.Err()
}

return roachpb.AdminScatterResponse{}, nil
return roachpb.AdminScatterResponse{
Ranges: []roachpb.AdminScatterResponse_Range{{
Span: roachpb.Span{
Key: desc.StartKey.AsRawKey(),
EndKey: desc.EndKey.AsRawKey(),
},
}},
}, nil
}

0 comments on commit 59abcf4

Please sign in to comment.