[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8551 )

Change subject: [spark] Remove AsyncClient in KuduContext
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG@7
PS1, Line 7: [spark] Remove AsyncClient in KuduContext
> Now we only update sync client for timestamp propagation and I don't see in
Indeed, I think a good compromise is to remove the sync cache, and change the 
sync method to

  def getSyncClient(kuduMaster: String): KuduClient = {
getAsyncClient(kuduMaster).syncClient()
  }



--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 21 Nov 2017 18:21:57 +
Gerrit-HasComments: Yes

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8551 )

Change subject: [spark] Remove AsyncClient in KuduContext
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG@7
PS1, Line 7: [spark] Remove AsyncClient in KuduContext
> Can we not just update both the sync and async clients? And propagate the m
Now we only update sync client for timestamp propagation and I don't see in 
this way it would affect the correctness? I thought the concern is to avoid 
misuse in future? (Maybe I have missed something)

If we are worrying about compatibility,  I remember Dan suggested another way 
is to redefine AsyncClient so that they are sharing the same client instance 
underneath.



--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 22:17:23 +
Gerrit-HasComments: Yes

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8551 )

Change subject: [spark] Remove AsyncClient in KuduContext
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG@7
PS1, Line 7: [spark] Remove AsyncClient in KuduContext
> I suggested the removal, since it's not used internally, and both this API
Can we not just update both the sync and async clients? And propagate the max 
from either one?



--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 21:40:09 +
Gerrit-HasComments: Yes

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8551 )

Change subject: [spark] Remove AsyncClient in KuduContext
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG@9
PS1, Line 9: avoid
> avoid
Done



--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 02:38:06 +
Gerrit-HasComments: Yes

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Hao Hao (Code Review)]

Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/8551

to look at the new patch set (#3).

Change subject: [spark] Remove AsyncClient in KuduContext
..

[spark] Remove AsyncClient in KuduContext

This patch removes asyncClient in KuduContext to avoid misuse, since
currently syncClient and asyncClient do not share the same AsyncKuduClient
instance.

Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
---
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/KuduContextTest.scala
2 files changed, 0 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/8551/3
--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8551 )

Change subject: [spark] Remove AsyncClient in KuduContext
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG@7
PS1, Line 7: [spark] Remove AsyncClient in KuduContext
> wouldn't this be a breaking change? I see we have annotated KuduContext as
I suggested the removal, since it's not used internally, and both this API and 
the AsyncKuduClient class itself are marked unstable.  Having two clients be 
available means that timestamp propagation doesn't work correctly.


http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG@9
PS1, Line 9: aviod
avoid



--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 00:44:53 +
Gerrit-HasComments: Yes

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8551 )

Change subject: [spark] Remove AsyncClient in KuduContext
..


Patch Set 2:

> Uploaded patch set 2: Patch Set 1 was rebased.

This is only a rebase.


--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 16 Nov 2017 00:14:40 +
Gerrit-HasComments: No

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8551 )

Change subject: [spark] Remove AsyncClient in KuduContext
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8551/1//COMMIT_MSG@7
PS1, Line 7: [spark] Remove AsyncClient in KuduContext
wouldn't this be a breaking change? I see we have annotated KuduContext as 
"Unstable" audience, but it seems a little bit harsh to remove a feature from 
it with no deprecation period, etc. Curious what Dan thinks about this.



--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 15 Nov 2017 22:50:01 +
Gerrit-HasComments: Yes

[kudu-CR] [spark] Remove AsyncClient in KuduContext

[Hao Hao (Code Review)]

Hao Hao has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8551


Change subject: [spark] Remove AsyncClient in KuduContext
..

[spark] Remove AsyncClient in KuduContext

This patch removes asyncClient in KuduContext to aviod misuse, since
currently syncClient and asyncClient do not share the same AsyncKuduClient
instance.

Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
---
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/KuduContextTest.scala
2 files changed, 0 insertions(+), 24 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/8551/1
--
To view, visit http://gerrit.cloudera.org:8080/8551
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ica8745d8503a35e17a632d0a0cde5738915f00fb
Gerrit-Change-Number: 8551
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao