You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org> on 2022/02/08 01:24:56 UTC
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Hongjiang Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18211
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Spark launches a speculative (duplicate) task for the long running task. If
the task runs deleting operations on kudu, it will cause 'key not found'
issue. This patch adds the basic functionality to support speculative
deleting tasks by adding deleteIgnoreRows.
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 57 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/1
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 1
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@334
PS4, Line 334: that have already been deleted.
> nit: remove this -- those absent rows might never be there in the first pla
Ack
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: ${numDeletes.value.get(tableName)}
> Is this going to be reported as an actual number of deleted rows or just th
This is the actual number of deleted rows. See the numDeletes here:
case Delete => numDeletes.add((tableName, count))
case DeleteIgnore => numDeletes.add((tableName, count))
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@74
PS4, Line 74: testDuplicateDelete
> In addition to this testcase with duplicated delete operations, maybe it's
Ack
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 4
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 14 Feb 2022 06:14:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 12
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 19:26:17 +0000
Gerrit-HasComments: No
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#2).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Spark launches a speculative (duplicate) task for the long running task. If
the task runs deleting operations on kudu, it will cause 'key not found'
issue. This patch adds the basic functionality to support speculative
deleting tasks by adding deleteIgnoreRows.
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 57 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/2
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 2
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 8: Code-Review+1
(2 comments)
Overall looks good to me! Just wanted to clarify on one small logging question.
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: ${numDeletes.value.get(tableName)}
> This is the actual number of deleted rows. See the numDeletes here:
Ah, that's interesting. Where does KuduContext get the stats about the number of actually deleted rows when it issues DELETE_IGNORE operations? Does the server side report to the client about that stat? It would be great if you add a comment clarifying on this, commenting about what's reported at the log.info() operator at line 347.
http://gerrit.cloudera.org:8080/#/c/18211/8/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala:
http://gerrit.cloudera.org:8080/#/c/18211/8/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@69
PS8, Line 69: * A simple test to delete rows from an empty table.
Thank you very much for adding this new scenario!
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 8
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 14 Feb 2022 22:03:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 10:
(1 comment)
> Patch Set 9:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: LETE_IGNORE operations successfull
> OK, now it makes sense to me: thank you for the confirmation!
Ack
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 10
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 02:59:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#3).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Spark launches a speculative (duplicate) task for the long running task. If
the task runs deleting operations on kudu, it will cause 'key not found'
issue. This patch adds the basic functionality to support speculative
deleting tasks by adding deleteIgnoreRows.
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/3
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 3
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#9).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 99 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/9
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 9
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#6).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 98 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/6
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 6
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 1:
Could you please take a look?
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 1
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Feb 2022 01:33:57 +0000
Gerrit-HasComments: No
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#4).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Spark launches a speculative (duplicate) task for the long running task. If
the task runs deleting operations on kudu, it will cause 'key not found'
issue. This patch adds the basic functionality to support speculative
deleting tasks by adding deleteIgnoreRows.
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 62 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/4
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 4
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/11/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/11/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@348
PS11, Line 348: deleted ${numDeletes.value.get(tableName)} rows from table '$tableName'
I meant this particular log line is confusing. It says that a particular number or rows have been deleted, but that's not true: imagine issuing 1000 DELETE_IGNORE operations to an empty table -- this would report that it has deleted 1000 rows, but in fact it deleted no rows at all.
Imagine somebody is looking into a file with these logs, thinking this line reports about the actual number of rows deleted (because it states doing so). It would be misleading.
Maybe, change this to be something like
"deleted up to ${numDeletes.value.get(tableName)} rows from table '$tableName' using DELETE_IGNORE"
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 11
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 06:45:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/11/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/11/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@348
PS11, Line 348: deleted ${numDeletes.value.get(tableName)} rows from table '$tableName'
> I meant this particular log line is confusing. It says that a particular n
Ah, and once you update the log message to be clear about the reported numbers, there is no need to have that comment at line 347.
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 11
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 06:46:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#8).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 98 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/8
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 8
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Reviewed-on: http://gerrit.cloudera.org:8080/18211
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 99 insertions(+), 0 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 13
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#5).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 98 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/5
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 5
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: umber of deleted rows instead of D
> Ah, here is the deleted rows from client's perspective or delete operations
OK, now it makes sense to me: thank you for the confirmation!
If that's not the actual number of deleted rows, the string logged might be confusing. Consider updating it to reflect that it's just the number of DELETE_IGNORE operations successfully processed, not the number of actually deleted rows.
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 9
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 02:56:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/11/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/11/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@348
PS11, Line 348: deleted ${numDeletes.value.get(tableName)} rows from table '$tableName'
> Ah, and once you update the log message to be clear about the reported numb
Ack
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 11
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 06:50:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#7).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 99 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/7
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 7
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#10).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 99 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/10
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 10
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: ${numDeletes.value.get(tableName)}
> Ah, that's interesting. Where does KuduContext get the stats about the num
The numDeletes Accumulator will increase after the KuduContext's writePartitionRows finishes. That is based on Spark's Accumulator2 interface.
What is about "the actual number of deleted rows instead of DELETE_IGNORE operations" for the comment?
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 8
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 02:28:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: ${numDeletes.value.get(tableName)}
> The numDeletes Accumulator will increase after the KuduContext's writeParti
Let me clarify: I'm not sure I understand how it's possible to report report the actual number of deleted rows when using DELETE_IGNORE. As I understand, the actual number of deleted rows is known only to the server side since it's not reported to the client. Do I miss anything?
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 4
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 02:35:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: umber of deleted rows instead of D
> Let me clarify: I'm not sure I understand how it's possible to report repor
Ah, here is the deleted rows from client's perspective or delete operations sent to the server, which may not be same as the actual deleted rows from server side.
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 9
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 15 Feb 2022 02:47:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#11).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 99 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/11
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 11
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18211
to look at the new patch set (#12).
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
[java] KUDU-3350 add the support for deleteIgnoreRows
Adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations
introduced with KUDU-1563(see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)
Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
---
M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
2 files changed, 99 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/18211/12
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 12
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [java] KUDU-3350 add the support for deleteIgnoreRows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18211 )
Change subject: [java] KUDU-3350 add the support for deleteIgnoreRows
......................................................................
Patch Set 4:
(4 comments)
Thank you for the patch!
http://gerrit.cloudera.org:8080/#/c/18211/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18211/4//COMMIT_MSG@9
PS4, Line 9: Spark launches a speculative (duplicate) task for the long running task. If
: the task runs deleting operations on kudu, it will cause 'key not found'
: issue. This patch adds the basic functionality to support speculative
: deleting tasks by adding deleteIgnoreRows.
I'm not sure I understand how it's relevant to talk about duplicated tasks to delete same rows. There might be many other scenarios which could send in a DELETE operation when the target row isn't present in the table.
Maybe, simply state that this patch adds a new deleteIgnoreRows() wrapper for DELETE_IGNORE operations introduced with KUDU-1563 (see https://github.com/apache/kudu/commit/7fbe341e51a9e4245d8b3017cecf11e393c3a22b)?
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@334
PS4, Line 334: that have already been deleted.
nit: remove this -- those absent rows might never be there in the first place, right?
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@347
PS4, Line 347: ${numDeletes.value.get(tableName)}
Is this going to be reported as an actual number of deleted rows or just the number of issued DELETE_IGNORE operations?
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala
File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala:
http://gerrit.cloudera.org:8080/#/c/18211/4/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@74
PS4, Line 74: testDuplicateDelete
In addition to this testcase with duplicated delete operations, maybe it's worth adding a very simple test to make sure that deleting anything from an empty table using DELETE_IGNORE always succeeds?
--
To view, visit http://gerrit.cloudera.org:8080/18211
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f89ced9ffa4a79f46661873f01c38aefb1d78d5
Gerrit-Change-Number: 18211
Gerrit-PatchSet: 4
Gerrit-Owner: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 12 Feb 2022 01:33:21 +0000
Gerrit-HasComments: Yes