You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/05/06 17:28:53 UTC
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13246
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
KUDU-2810: [backup] Add workaround DELETE IGNORE support
This patch adds a client side version of DELETE IGNORE
for use in the restore job. It does this the same way
existing client side INSERT IGNORE support was added,
by checking and filtering the error code client side.
More optimal server side ignore functionality is tracked
by KUDU-1563.
Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
8 files changed, 162 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/13246/1
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 21:09:06 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java:
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java@70
PS2, Line 70: Set<ErrorCode> ignoredErrors = new HashSet<>();
ErrorCode is defined as an enum in protobuf, so can this be an EnumSet?
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java:
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@159
PS2, Line 159:
Nit: extra space here. Same in setIgnoreAllNotFoundRows().
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@185
PS2, Line 185: void setIgnoreAllNotFoundRows(boolean ignoreAllNotFoundRows);
In your opinion do these new APIs make it more difficult to implement KUDU-1563?
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java:
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java@113
PS2, Line 113: public void testIgnoreAllNotFoundRows() throws Exception {
This is a weird structure for this unit test:
1. Why bother testing multiple flush modes? There doesn't appear to be any intersection between the functionality being tested and the flush mode. It leads to weird test code like sleeping until AUTO_FLUSH_BACKGROUND is done.
2. Seems like a minimal test would just insert a row, then delete it three times:
2a. First delete succeeds.
2b. Second delete returns a row error. Then you change the session configuration.
2c. Third delete succeeds. Maybe repeat the whole sequence for two rows if you want to test that the configuration doesn't somehow only apply to only one row.
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 18:01:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 17:56:49 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 3:
For the record I tested this on a real cluster as follows:
1. Insert 100 rows.
2. Backup.
3. Insert 100 rows.
4. Delete 1/7 of all rows.
5. Backup.
6. Restore from backups.
Before this patch, the restore failed. With this patch, it succeeds.
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 20:04:25 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@101
PS1, Line 101:
> nit: Extra space.
This is on purpose, it allows IDEs to detect a multiline todo.
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@421
PS1, Line 421: // When restoring the table, delete half the rows after each job completes.
: // This will force delete rows to cause NotFound errors and allow validation
: // that they are correctly handled.
> Can't we reproduce this without doing something fancy?
I don't want this test to depend on the "bad" diff scan behavior because Adar is working on fixing that. Even after diff scan doesn't included incorrect deletes we need to ensure delete ignore works for retries.
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java:
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java@70
PS2, Line 70: Set<ErrorCode> ignoredErrors = new HashSet<>();
> ErrorCode is defined as an enum in protobuf, so can this be an EnumSet?
Done
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@182
PS1, Line 182: <p>
> What's the purpose of this tag?
I copied it from above. It appears a couple times in the file. It just formats the javadoc html presentation slightly.
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java:
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@159
PS2, Line 159:
> Nit: extra space here. Same in setIgnoreAllNotFoundRows().
This is on purpose, it allows IDEs to detect a multiline todo.
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@185
PS2, Line 185: void setIgnoreAllNotFoundRows(boolean ignoreAllNotFoundRows);
> In your opinion do these new APIs make it more difficult to implement KUDU-
I don't think so. Technically both, ignoring errors and preventing errors, can exist and work together regardless of how we implement them.
If IgnoreAllDuplicateRows functionality didn't already I would lean towards not adding this. Primarily because the performance of relying on filtering per-row errors is likely to be subpar. But because it does already exist, adding another instance of ignoring error codes doesn't add any more complexity or problems.
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java@118
PS1, Line 118: for (SessionConfiguration.FlushMode mode : SessionConfiguration.FlushMode.values()) {
> A one-sentence comment on why it's necessary to test all the flush modes wo
Done
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java:
http://gerrit.cloudera.org:8080/#/c/13246/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java@113
PS2, Line 113: public void testIgnoreAllNotFoundRows() throws Exception {
> This is a weird structure for this unit test:
This is primarily a "copy" of the test logic for testIgnoreAllDuplicateRows. Per Wills review I added a comment explaining the motivation for testing each flush type.
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 18:28:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
KUDU-2810: [backup] Add workaround DELETE IGNORE support
This patch adds a client side version of DELETE IGNORE
for use in the restore job. It does this the same way
existing client side INSERT IGNORE support was added,
by checking and filtering the error code client side.
More optimal server side ignore functionality is tracked
by KUDU-1563.
Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Reviewed-on: http://gerrit.cloudera.org:8080/13246
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Grant Henke <gr...@apache.org>
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
8 files changed, 172 insertions(+), 23 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Grant Henke: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13246/3/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java:
http://gerrit.cloudera.org:8080/#/c/13246/3/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java@65
PS3, Line 65: private final EnumSet<ErrorCode> ignoredErrors;
> I think you could continue to use an interface type (i.e. Set) here.
Since it's private and the fact that it's a EnumSet is important I decided to use the implementation type.
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 18:42:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13246/3/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java:
http://gerrit.cloudera.org:8080/#/c/13246/3/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java@65
PS3, Line 65: private final EnumSet<ErrorCode> ignoredErrors;
I think you could continue to use an interface type (i.e. Set) here.
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 18:36:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 5: Verified+1 Code-Review+2
Carrying +2 through a trivial rebase.
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 22:09:16 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13246
to look at the new patch set (#4).
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
KUDU-2810: [backup] Add workaround DELETE IGNORE support
This patch adds a client side version of DELETE IGNORE
for use in the restore job. It does this the same way
existing client side INSERT IGNORE support was added,
by checking and filtering the error code client side.
More optimal server side ignore functionality is tracked
by KUDU-1563.
Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
8 files changed, 172 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/13246/4
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 20:04:38 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala@101
PS1, Line 101:
nit: Extra space.
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@421
PS1, Line 421: // When restoring the table, delete half the rows after each job completes.
: // This will force delete rows to cause NotFound errors and allow validation
: // that they are correctly handled.
Can't we reproduce this without doing something fancy?
1. Do a backup (an empty one)
2. Insert a row.
3. Delete the row.
4. Do an incremental backup
5. Try to restore the table.
The row deleted in step 4 will generate a delete operation during restore but the row won't be present in the table as restored from the backup in step 2.
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
File java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java@182
PS1, Line 182: <p>
What's the purpose of this tag?
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java:
http://gerrit.cloudera.org:8080/#/c/13246/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java@118
PS1, Line 118: for (SessionConfiguration.FlushMode mode : SessionConfiguration.FlushMode.values()) {
A one-sentence comment on why it's necessary to test all the flush modes would be nice. My understanding from reading the patch is that some flush modes flush individual operations and some flush batches, and each has some of its own separate handling of per-row errors.
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 17:58:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has removed a vote on this change.
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13246
to look at the new patch set (#5).
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
KUDU-2810: [backup] Add workaround DELETE IGNORE support
This patch adds a client side version of DELETE IGNORE
for use in the restore job. It does this the same way
existing client side INSERT IGNORE support was added,
by checking and filtering the error code client side.
More optimal server side ignore functionality is tracked
by KUDU-1563.
Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
8 files changed, 172 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/13246/5
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13246
to look at the new patch set (#3).
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
KUDU-2810: [backup] Add workaround DELETE IGNORE support
This patch adds a client side version of DELETE IGNORE
for use in the restore job. It does this the same way
existing client side INSERT IGNORE support was added,
by checking and filtering the error code client side.
More optimal server side ignore functionality is tracked
by KUDU-1563.
Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
---
M java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala
M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
8 files changed, 167 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/13246/3
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2810: [backup] Add workaround DELETE IGNORE support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13246 )
Change subject: KUDU-2810: [backup] Add workaround DELETE IGNORE support
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13246
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib718b72a73b06d9b9dc809fde3c52a1d498ceb23
Gerrit-Change-Number: 13246
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 22:09:11 +0000
Gerrit-HasComments: No