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

workload: use IMPORT then BACKUP in 'fixtures make' #37343

Merged
merged 1 commit into from
May 7, 2019

Conversation

dt
Copy link
Member

@dt dt commented May 6, 2019

Previously we used the 'transform' option on IMPORT.

However that option will soon go away as IMPORT is increasingly focused on ingestion.
Making fixtures should hopefully be rare enough that we can do a little extra work here
rather than maintain the special 'transform' mode.

Release note: none.

@dt dt requested review from danhhz, bobvawter and a team May 6, 2019 22:45
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@bobvawter bobvawter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @danhhz)

Copy link
Contributor

@danhhz danhhz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fixtures make command didn't used to modify your cluster (besides creating jobs). I'd add a very short bit to the cli cmd description that it works by ingesting and backing up the data.

:lgtm: mod 2 comments

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @dt)


pkg/ccl/workloadccl/fixture.go, line 264 at r1 (raw file):

	}

	if _, err := ImportFixture(ctx, sqlDB, gen, dbName, false, 1, false, ""); err != nil {

our style is to have /* */ comments on all these false, 1, false, "" params

Previously we used the 'transform' option on IMPORT.

However that option will soon go away as IMPORT is increasingly focused on ingestion.
Making fixtures should hopefully be rare enough that we can do a little extra work here
rather than maintain the special 'transform' mode.

Release note: none.
@dt dt force-pushed the fixtures-make-via-backup branch from c024660 to f4a0350 Compare May 7, 2019 14:45
Copy link
Member Author

@dt dt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @bobvawter)


pkg/ccl/workloadccl/fixture.go, line 264 at r1 (raw file):

Previously, danhhz (Daniel Harrison) wrote…

our style is to have /* */ comments on all these false, 1, false, "" params

Done.

I made them named consts.

Copy link
Contributor

@danhhz danhhz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @bobvawter)


pkg/ccl/workloadccl/fixture.go, line 264 at r1 (raw file):

Previously, dt (David Taylor) wrote…

Done.

I made them named consts.

Even better

@dt
Copy link
Member Author

dt commented May 7, 2019

bors r+

@dt
Copy link
Member Author

dt commented May 7, 2019

TFTRs

craig bot pushed a commit that referenced this pull request May 7, 2019
37343: workload: use IMPORT then BACKUP in 'fixtures make' r=dt a=dt

Previously we used the 'transform' option on IMPORT.

However that option will soon go away as IMPORT is increasingly focused on ingestion.
Making fixtures should hopefully be rare enough that we can do a little extra work here
rather than maintain the special 'transform' mode.

Release note: none.

Co-authored-by: David Taylor <tinystatemachine@gmail.com>
@craig
Copy link
Contributor

craig bot commented May 7, 2019

Build succeeded

@craig craig bot merged commit f4a0350 into cockroachdb:master May 7, 2019
@dt dt deleted the fixtures-make-via-backup branch May 8, 2019 20:26
danhhz added a commit to danhhz/cockroach that referenced this pull request May 21, 2019
The SQL database for all the tables in the BACKUPs created by `fixtures
make` used to be "csv" (an artifact of the way we made them), but as
of cockroachdb#37343 it's the name of the generator. This seems better so change
`fixtures load` to match.

The same PR also (accidentally) started adding foreign keys in the
BACKUPs, but since there's one table per BACKUP (another artifact of the
way we used to make fixtures), we can't restore the foreign keys. It'd
be nice to switch to one BACKUP with all tables and get the foreign
keys, but the UX of the postLoad hook becomes tricky and I don't have
time right now to sort it all out. So, revert to the previous behavior
(no fks in fixtures) for now.

Release note: None
craig bot pushed a commit that referenced this pull request May 21, 2019
37668: storage: fix and test a bogus source of replica divergence errors r=nvanbenschoten a=tbg

An incompatibility in the consistency checks was introduced between v2.1 and v19.1.
See individual commit messages and #37425 for details.

Release note (bug fix): Fixed a potential source of (faux) replica
inconsistencies that can be reported while running a mixed v19.1 / v2.1
cluster. This error (in that situation only) is benign and can be
resolved by upgrading to the latest v19.1 patch release. Every time this
error occurs a "checkpoint" is created which will occupy a large amount
of disk space and which needs to be removed manually (see <store
directory>/auxiliary/checkpoints).

Release note (bug fix): Fixed a case in which `./cockroach quit` would
return success even though the server process was still running in a
severely degraded state.

37701: workloadcccl: fix two regressions in fixtures make/load r=nvanbenschoten a=danhhz

The SQL database for all the tables in the BACKUPs created by `fixtures
make` used to be "csv" (an artifact of the way we made them), but as
of #37343 it's the name of the generator. This seems better so change
`fixtures load` to match.

The same PR also (accidentally) started adding foreign keys in the
BACKUPs, but since there's one table per BACKUP (another artifact of the
way we used to make fixtures), we can't restore the foreign keys. It'd
be nice to switch to one BACKUP with all tables and get the foreign
keys, but the UX of the postLoad hook becomes tricky and I don't have
time right now to sort it all out. So, revert to the previous behavior
(no fks in fixtures) for now.

Release note: None

Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Co-authored-by: Daniel Harrison <daniel.harrison@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants