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.

Sign up for GitHub

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

Jump to bottom

roachtest: add DMS tests for no PK case #95328

Closed
otan opened this issue Jan 16, 2023 · 0 comments · Fixed by #95518
Closed

roachtest: add DMS tests for no PK case #95328

otan opened this issue Jan 16, 2023 · 0 comments · Fixed by #95518
Assignees
@Jeremyyang920
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@otan
Copy link
Contributor

otan commented Jan 16, 2023 •
edited by cockroach-jira-scripts
Loading

We are about to recommend users (a) have atomic copy disabled and (b) have retries enabled for COPY.

This is presumed to be fine as we strongly believe DMS it doesn't retry. But if DMS does retry, we're concerned with the above conditions introducing duplicate rows. This is naturally caught if a PK constraint exists.

For the no PK case, we'd like to add regression tests. We should add a DMS test that verifies that if we try to DMS into a no-PK table:

  • Drop table on target silently fails with an error.
  • Truncate silently fails with an error.

As of writing, I can verify DMS does the above behaviour at the moment.

Jira issue: CRDB-23460
@otan otan added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jan 16, 2023
@Jeremyyang920 Jeremyyang920 mentioned this issue Jan 19, 2023
Merged
craig bot pushed a commit that referenced this issue Jan 19, 2023
@Jeremyyang920 @ecwall @irfansharif
Merge #95518 #95526 #95536
ba572fc 
95518: roachtest/awsdms: add no pk full load test case r=Jeremyyang920 a=Jeremyyang920

This commit adds a new test case that will attempt a full load on a table with no primary key. We will
assert that there is a table error with the full
load.

Fixes: #95328

Release note: None

95526: multitenant: append `WITH REPLICATION STATUS` columns to `SHOW TENANT` columns r=knz a=ecwall

Informs #87851

`WITH CAPABILITIES` will be adding another set of columns so append relevant columns instead of creating all possible column sets ahead of time.

Release note: None

95536: grunning: add grunning.Elapsed() r=irfansharif a=irfansharif

Elapsed returns the running time spent doing some piece of work, with grunning.Time() measurements from the start and end. This only exists due to grunning.Time()'s non-monotonicity, a bug in our runtime patch: #95529. We can get rid of this, keeping just grunning.Difference(), if that bug is fixed. The bug results in slight {over,under}-estimation of the running time (the latter breaking monotonicity), but is livable with our current uses of this library.

Release note: None

Co-authored-by: Jeremy Yang <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
@craig craig bot closed this as completed in 1d59169 Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Jeremyyang920 Jeremyyang920

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

roachtest/awsdms: add no pk full load test case Jeremyyang920/cockroach
2 participants
@otan @Jeremyyang920