Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workloadcccl: fix two regressions in fixtures make/load #37701

Merged
merged 1 commit into from
May 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/ccl/workloadccl/allccl/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const (
directIngestion = true
oneFilePerNode = 1
noInjectStats = false
noSkipPostLoad = false
skipCSVRoundtrip = ``
)

Expand Down Expand Up @@ -84,7 +85,8 @@ func TestAllRegisteredImportFixture(t *testing.T) {
sqlutils.MakeSQLRunner(db).Exec(t, `CREATE DATABASE d`)

if _, err := workloadccl.ImportFixture(
ctx, db, gen, `d`, directIngestion, oneFilePerNode, noInjectStats, skipCSVRoundtrip,
ctx, db, gen, `d`, directIngestion, oneFilePerNode, noInjectStats, noSkipPostLoad,
skipCSVRoundtrip,
); err != nil {
t.Fatal(err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/workloadccl/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ func benchmarkImportFixture(b *testing.B, gen workload.Generator) {

b.StartTimer()
const filesPerNode = 1
const directIngest, noInjectStats, skipPostLoad, csvServer = true, false, true, ``
importBytes, err := ImportFixture(
ctx, db, gen, `d`, true /* directIngestion */, filesPerNode, false, /* injectStats */
``, /* csvServer */
ctx, db, gen, `d`, directIngest, filesPerNode, noInjectStats, skipPostLoad, csvServer,
)
require.NoError(b, err)
bytes += importBytes
Expand Down
3 changes: 2 additions & 1 deletion pkg/ccl/workloadccl/cliccl/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,10 @@ func fixturesImport(gen workload.Generator, urls []string, dbName string) error
directIngestion := *fixturesImportDirectIngestionTable
filesPerNode := *fixturesImportFilesPerNode
injectStats := *fixturesImportInjectStats
noSkipPostLoad := false
csvServer := *fixturesMakeImportCSVServerURL
bytes, err := workloadccl.ImportFixture(
ctx, sqlDB, gen, dbName, directIngestion, filesPerNode, injectStats, csvServer,
ctx, sqlDB, gen, dbName, directIngestion, filesPerNode, injectStats, noSkipPostLoad, csvServer,
)
if err != nil {
return errors.Wrap(err, `importing fixture`)
Expand Down
18 changes: 11 additions & 7 deletions pkg/ccl/workloadccl/fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,10 @@ func MakeFixture(
if _, err := sqlDB.Exec(`CREATE DATABASE IF NOT EXISTS ` + dbName); err != nil {
return Fixture{}, err
}
const direct, stats, csvServer = false, false, ""
if _, err := ImportFixture(ctx, sqlDB, gen, dbName, direct, filesPerNode, stats, csvServer); err != nil {
const direct, stats, skipPostLoad, csvServer = false, false, true, ""
if _, err := ImportFixture(
ctx, sqlDB, gen, dbName, direct, filesPerNode, stats, skipPostLoad, csvServer,
); err != nil {
return Fixture{}, err
}
g := ctxgroup.WithContext(ctx)
Expand Down Expand Up @@ -293,6 +295,7 @@ func ImportFixture(
directIngestion bool,
filesPerNode int,
injectStats bool,
skipPostLoad bool,
csvServer string,
) (int64, error) {
for _, t := range gen.Tables() {
Expand Down Expand Up @@ -339,8 +342,10 @@ func ImportFixture(
if err := g.Wait(); err != nil {
return 0, err
}
if err := runPostLoadSteps(ctx, sqlDB, gen); err != nil {
return 0, err
if !skipPostLoad {
if err := runPostLoadSteps(ctx, sqlDB, gen); err != nil {
return 0, err
}
}
return atomic.LoadInt64(&bytesAtomic), nil
}
Expand Down Expand Up @@ -449,13 +454,12 @@ func RestoreFixture(
) (int64, error) {
var bytesAtomic int64
g := ctxgroup.WithContext(ctx)
genName := fixture.Generator.Meta().Name
for _, table := range fixture.Tables {
table := table
g.GoCtx(func(ctx context.Context) error {
// The IMPORT ... CSV DATA command generates a backup with the table in
// database `csv`.
start := timeutil.Now()
importStmt := fmt.Sprintf(`RESTORE csv.%s FROM $1 WITH into_db=$2`, table.TableName)
importStmt := fmt.Sprintf(`RESTORE %s.%s FROM $1 WITH into_db=$2`, genName, table.TableName)
var rows, index, tableBytes int64
var discard interface{}
if err := sqlDB.QueryRow(importStmt, table.BackupURI, database).Scan(
Expand Down
8 changes: 5 additions & 3 deletions pkg/ccl/workloadccl/fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,11 @@ func TestImportFixture(t *testing.T) {
}

const filesPerNode = 1
const noSkipPostLoad = false
sqlDB.Exec(t, `CREATE DATABASE distsort`)
_, err := ImportFixture(
ctx, db, gen, `distsort`, false /* directIngestion */, filesPerNode, true, /* injectStats */
``, /* csvServer */
noSkipPostLoad, ``, /* csvServer */
)
require.NoError(t, err)
sqlDB.CheckQueryResults(t,
Expand All @@ -203,7 +204,7 @@ func TestImportFixture(t *testing.T) {
sqlDB.Exec(t, `CREATE DATABASE direct`)
_, err = ImportFixture(
ctx, db, gen, `direct`, true /* directIngestion */, filesPerNode, false, /* injectStats */
``, /* csvServer */
noSkipPostLoad, ``, /* csvServer */
)
require.NoError(t, err)
sqlDB.CheckQueryResults(t,
Expand Down Expand Up @@ -240,9 +241,10 @@ func TestImportFixtureCSVServer(t *testing.T) {
}

const filesPerNode = 1
const noDirectIngest, noInjectStats, noSkipPostLoad = false, false, true
sqlDB.Exec(t, `CREATE DATABASE d`)
_, err := ImportFixture(
ctx, db, gen, `d`, false /* directIngestion */, filesPerNode, false /* injectStats */, ts.URL,
ctx, db, gen, `d`, noDirectIngest, filesPerNode, noInjectStats, noSkipPostLoad, ts.URL,
)
require.NoError(t, err)
sqlDB.CheckQueryResults(t,
Expand Down