[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Internal Jenkins (Code Review)
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Internal Jenkins
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Internal Jenkins (Code Review)
Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Reviewed-on: http://gerrit.cloudera.org:8080/5364
Reviewed-by: Dimitris Tsirogiannis 
Tested-by: Internal Jenkins
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Dimitris Tsirogiannis: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Internal Jenkins


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 3: Code-Review+2

Carry Alex's +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Dimitris Tsirogiannis (Code Review)
Hello Alex Behm,

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

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

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

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..

IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/5364/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5364/2/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 296: throw new ImpalaRuntimeException(errMsg);
> add "timed out blurb"
ha forgot that. Thanks :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5364/2/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 296: throw new ImpalaRuntimeException(errMsg);
add "timed out blurb"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has uploaded a new patch set (#2).

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..

IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/5364/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5364/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 432:* the alter table operation is finished or until the operation 
timeout is reached.
> I think we should distinguish those cases for supportability reasons.
Good point. Modified the error message for the case of a timeout.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5364/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 432:* the alter table operation is finished or until the operation 
timeout is reached.
> If the function returns false it means it timed out.
I think we should distinguish those cases for supportability reasons.


Line 435:   public static void alterKuduTable(KuduTable tbl, AlterTableOptions 
ato, String errMsg)
> Hm, not sure how this would work. For the rename you need to call the opera
Ahh you are right, got it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5364/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 432:* the alter table operation is finished or until the operation 
timeout is reached.
> is there a way for us to distinguish failure from timeout?
If the function returns false it means it timed out.


Line 435:   public static void alterKuduTable(KuduTable tbl, AlterTableOptions 
ato, String errMsg)
> pass name and master hosts list? then you can use this for the renameTable(
Hm, not sure how this would work. For the rename you need to call the operation 
on the old name but check (call isAlterTableDone) on the new name.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Alex Behm (Code Review)
Alex Behm has posted comments on this change.

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5364/1/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java:

Line 432:* the alter table operation is finished or until the operation 
timeout is reached.
is there a way for us to distinguish failure from timeout?


Line 435:   public static void alterKuduTable(KuduTable tbl, AlterTableOptions 
ato, String errMsg)
pass name and master hosts list? then you can use this for the renameTable() 
case as well


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4584: Make alter table operations on Kudu tables synchronous

2016-12-05 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has uploaded a new change for review.

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

Change subject: IMPALA-4584: Make alter table operations on Kudu tables 
synchronous
..

IMPALA-4584: Make alter table operations on Kudu tables synchronous

This commit changes the behavior of alter table operations on Kudu
tables from asynchronous to synchronous. With this change, alter table
operations return when either the operations complete successfully or
a timeout is reached.

Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
---
M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_alter.test
2 files changed, 36 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/5364/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I385bce66691ae9040e72f97557e1bba31009e36b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis