[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-09 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 09 May 2018 22:27:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-09 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..

IMPALA-6948,IMPALA-6962: add end-to-end tests

Adds end-to-end tests to validate that following
various metadata operations, the catalog state
in catalogd and impalads is the same.

For IMPALA-6962, catalogd process restart for tests
is fixed.

Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Reviewed-on: http://gerrit.cloudera.org:8080/10291
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins 
---
M be/src/catalog/catalog-server.cc
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_metadata_replicas.py
M tests/metadata/test_hms_integration.py
A tests/util/hive_utils.py
6 files changed, 242 insertions(+), 68 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 7
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-09 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2435/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 09 May 2018 18:45:53 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-09 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 09 May 2018 18:25:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-09 Thread Vuk Ercegovac (Code Review) 
Hello Dimitris Tsirogiannis, Alex Behm,

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

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

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

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..

IMPALA-6948,IMPALA-6962: add end-to-end tests

Adds end-to-end tests to validate that following
various metadata operations, the catalog state
in catalogd and impalads is the same.

For IMPALA-6962, catalogd process restart for tests
is fixed.

Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
---
M be/src/catalog/catalog-server.cc
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_metadata_replicas.py
M tests/metadata/test_hms_integration.py
A tests/util/hive_utils.py
6 files changed, 242 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/10291/6
--
To view, visit http://gerrit.cloudera.org:8080/10291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-09 Thread Vuk Ercegovac (Code Review) 
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc@304
PS3, Line 304: void CatalogServer::CatalogUrlCallback(const 
Webserver::ArgumentMap& args,
> I just think it's weird that we don't show the dbs if there was an error wi
done. makes sense, I was treating it as all-or-nothing.


http://gerrit.cloudera.org:8080/#/c/10291/5/tests/custom_cluster/test_metadata_replicas.py
File tests/custom_cluster/test_metadata_replicas.py:

http://gerrit.cloudera.org:8080/#/c/10291/5/tests/custom_cluster/test_metadata_replicas.py@50
PS5, Line 50:   self.client.execute("invalidate metadata 
functional.alltypes")
> check that the catalog version is at least 50 at this point to make debuggi
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 09 May 2018 18:23:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-09 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc@304
PS3, Line 304: void CatalogServer::CatalogUrlCallback(const 
Webserver::ArgumentMap& args,
> did it this way to share the error handling and avoid multiple "error" fiel
I just think it's weird that we don't show the dbs if there was an error with 
getting the catalog version. We return in L315 even though the dbs are just 
fine. Might make debugging slightly harder if something goes wrong.

Don't think we need to do something too complicated, just want to be sure we 
can easily pinpoint the error if something does wrong.


http://gerrit.cloudera.org:8080/#/c/10291/5/tests/custom_cluster/test_metadata_replicas.py
File tests/custom_cluster/test_metadata_replicas.py:

http://gerrit.cloudera.org:8080/#/c/10291/5/tests/custom_cluster/test_metadata_replicas.py@50
PS5, Line 50:   self.client.execute("invalidate metadata 
functional.alltypes")
check that the catalog version is at least 50 at this point to make debugging 
easier if something goes wrong (e.g. we  purposely or accidentally change no-op 
ddls to not increment the version)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 09 May 2018 18:08:34 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-08 Thread Vuk Ercegovac (Code Review) 
Hello Dimitris Tsirogiannis, Alex Behm,

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

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

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

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..

IMPALA-6948,IMPALA-6962: add end-to-end tests

Adds end-to-end tests to validate that following
various metadata operations, the catalog state
in catalogd and impalads is the same.

For IMPALA-6962, catalogd process restart for tests
is fixed.

Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
---
M be/src/catalog/catalog-server.cc
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_metadata_replicas.py
M tests/metadata/test_hms_integration.py
A tests/util/hive_utils.py
6 files changed, 240 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/10291/5
--
To view, visit http://gerrit.cloudera.org:8080/10291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 5
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-08 Thread Vuk Ercegovac (Code Review) 
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 4:

changed how catalogd process is restarted: now using os.system instead of the 
wrapper around popen. that change fixes an issue with zombie procs in the 
jenkins env (that unfortunately was not present in the dev env).


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 09 May 2018 05:35:54 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-08 Thread Vuk Ercegovac (Code Review) 
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc@304
PS3, Line 304: long current_catalog_version;
> Why is this dependent on the success of GetDbs()? I think the version and t
did it this way to share the error handling and avoid multiple "error" fields. 
I could have an error field and a list of reasons?


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/common/impala_service.py
File tests/common/impala_service.py:

http://gerrit.cloudera.org:8080/#/c/10291/3/tests/common/impala_service.py@151
PS3, Line 151: """ Extracts and returns the version of the catalog object's 
'thrift_txt' representation."""
> I think the JSON field is called 'thrift_string'
the value that passed in here is coming from get_catalog_object_dump, which is 
just html, not json.


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py
File tests/custom_cluster/test_metadata_replicas.py:

http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@39
PS3, Line 39:   @pytest.mark.xfail(reason="IMPALA-6948")
> Issue is fixed now.
Done


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@54
PS3, Line 54:   "create table if not exists %s.x (a string)" % db_name)
> We should not do IF NOT EXISTS here and IF EXISTS below in the drop. If the
Done


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@55
PS3, Line 55: cursor.execute("set SYNC_DDL=true")
> Not obvious why we need this. Can we remove it if we consistently use the s
Done


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@57
PS3, Line 57: assert "x" in self.client.execute("show tables in %s" % 
db_name).data
> The use of both cursor and self.client is somewhat confusing. How many conn
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 09 May 2018 00:26:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-08 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 3:

(6 comments)

I'm pretty happy with this test.

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:

http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc@304
PS3, Line 304: long current_catalog_version;
Why is this dependent on the success of GetDbs()? I think the version and the 
dbs should be handled independently.


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/common/impala_service.py
File tests/common/impala_service.py:

http://gerrit.cloudera.org:8080/#/c/10291/3/tests/common/impala_service.py@151
PS3, Line 151: """ Extracts and returns the version of the catalog object's 
'thrift_txt' representation."""
I think the JSON field is called 'thrift_string'


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py
File tests/custom_cluster/test_metadata_replicas.py:

http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@39
PS3, Line 39:   @pytest.mark.xfail(reason="IMPALA-6948")
Issue is fixed now.


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@54
PS3, Line 54:   "create table if not exists %s.x (a string)" % db_name)
We should not do IF NOT EXISTS here and IF EXISTS below in the drop. If the 
object unexpectedly exists / does not exist then this test might fail and be 
hard to debug.


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@55
PS3, Line 55: cursor.execute("set SYNC_DDL=true")
Not obvious why we need this. Can we remove it if we consistently use the same 
impalad client?


http://gerrit.cloudera.org:8080/#/c/10291/3/tests/custom_cluster/test_metadata_replicas.py@57
PS3, Line 57: assert "x" in self.client.execute("show tables in %s" % 
db_name).data
The use of both cursor and self.client is somewhat confusing. How many 
connections/sessions/clients are there? Better to avoid those thoughts and 
stick with one.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 08 May 2018 23:42:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-08 Thread Vuk Ercegovac (Code Review) 
Hello Dimitris Tsirogiannis, Alex Behm,

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

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

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

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..

IMPALA-6948,IMPALA-6962: add end-to-end tests

Adds end-to-end tests to validate that following
various metadata operations, the catalog state
in catalogd and impalads is the same.

For IMPALA-6962, catalogd process restart for tests
is fixed.

Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
---
M be/src/catalog/catalog-server.cc
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_metadata_replicas.py
M tests/metadata/test_hms_integration.py
A tests/util/hive_utils.py
6 files changed, 243 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-08 Thread Vuk Ercegovac (Code Review) 
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py
File tests/custom_cluster/test_metadata_replicas.py:

http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@39
PS2, Line 39: def test_load_all(self, cursor):
: """ Loads metadata for all tables and validates that they're 
all the same."""
: # Use describe to issue table loads.
: c_objects = 
self.cluster.catalogd.service.get_catalog_objects()
: for obj in c_objects:
:   if obj[1] == "TABLE": cursor.execute("describe %s" % obj[0])
: # Use a sentinel ddl with sync_ddl to make sure that all 
impalads
: # have receieved the effect of the previous ddl's.
: cursor.execute("set sync_ddl=true")
: cursor.execute("invalidate metadata functional.alltypes")
:
: self.__validate_metadata()
> This test is going to be very expensive and I am not sure how much extra co
Removing this test... alex raised concerns about expense as well. It takes 
about 7-8 s, which is still less than 50% of the total time (remaining time 
needed for custom cluster setup/teardown.
However, since its not the main point of this change, I'll remove it; we can 
add something like this later on if needed.


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@69
PS2, Line 69: cursor.execute("invalidate metadata")
> No need for the global one, you may do "invalidate metadata x".
Done


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@79
PS2, Line 79: wait_time_s = specific_build_type_timeout(10, 
slow_build_timeout=20)
: sleep(wait_time_s)
> I agree with the TODO and I think we should fix it now. Essentially, in ord
Added a catalog version to the /catalog page.
For this test, I check the catalog version and the latest catalog version from 
each impalad. When they are in agreement, the rest of the test can proceed.
When starting up the catalogd, we wait until its subscribed to the statestore. 
Since subscription happens *after* the catalog is instantiated and loaded, its 
safe to access the catalogd catalog version at this point.


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@81
PS2, Line 81: self.cluster.refresh()
> Hm, why is this needed?
refreshes the cluster process list after restarting a new catalogd process. 
needed to kill it later on L88.


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@123
PS2, Line 123: def __dump_objects(self, catalog, impalads):
 : """ For debugging, prints all objects and their version."""
 : print "CATALOG OBJECTS"
 : for k, v in catalog.items():
 :   print "%s, %s, %d" % (k, v[0], v[1])
 :
 : for idx in xrange(0,len(impalads)):
 :   print "IMPALAD %d OBJECTS" % idx
 :   for k, v in impalads[idx].items():
 : print "%s, %s, %d" % (k, v[0], v[1])
> Is this used anywhere?
just using it for debugging so removed.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 08 May 2018 20:32:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-07 Thread Dimitris Tsirogiannis (Code Review) 
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py
File tests/custom_cluster/test_metadata_replicas.py:

http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@39
PS2, Line 39: def test_load_all(self, cursor):
: """ Loads metadata for all tables and validates that they're 
all the same."""
: # Use describe to issue table loads.
: c_objects = 
self.cluster.catalogd.service.get_catalog_objects()
: for obj in c_objects:
:   if obj[1] == "TABLE": cursor.execute("describe %s" % obj[0])
: # Use a sentinel ddl with sync_ddl to make sure that all 
impalads
: # have receieved the effect of the previous ddl's.
: cursor.execute("set sync_ddl=true")
: cursor.execute("invalidate metadata functional.alltypes")
:
: self.__validate_metadata()
This test is going to be very expensive and I am not sure how much extra 
coverage it adds compared to our existing metadata tests. Also, if we decide to 
keep it, you may want to consider using the load_catalog_in_background flag to 
trigger metadata load for all tables instead of running describe for each one 
of them.


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@69
PS2, Line 69: cursor.execute("invalidate metadata")
No need for the global one, you may do "invalidate metadata x".


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@79
PS2, Line 79: wait_time_s = specific_build_type_timeout(10, 
slow_build_timeout=20)
: sleep(wait_time_s)
I agree with the TODO and I think we should fix it now. Essentially, in order 
to ensure that we are in steady state and that the coordinators have been 
through the cycle of detecting the catalog restart, asking the full update and 
receiving it, we need to compare the current value of the catalog version in 
the catalog and the last received catalog version in the coordinator (the 
latter is already a metric).


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@81
PS2, Line 81: self.cluster.refresh()
Hm, why is this needed?


http://gerrit.cloudera.org:8080/#/c/10291/2/tests/custom_cluster/test_metadata_replicas.py@123
PS2, Line 123: def __dump_objects(self, catalog, impalads):
 : """ For debugging, prints all objects and their version."""
 : print "CATALOG OBJECTS"
 : for k, v in catalog.items():
 :   print "%s, %s, %d" % (k, v[0], v[1])
 :
 : for idx in xrange(0,len(impalads)):
 :   print "IMPALAD %d OBJECTS" % idx
 :   for k, v in impalads[idx].items():
 : print "%s, %s, %d" % (k, v[0], v[1])
Is this used anywhere?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 07 May 2018 19:15:05 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-03 Thread Vuk Ercegovac (Code Review) 
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10291 )

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..


Patch Set 2:

(1 comment)

several changes:
- downgraded several lines to python 2.6
- ran into trouble with unique_database fixture + custom cluster when
  explicitly killing catalogd so I factored out a wrapper from another test
  that handles setup/cleanup.

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

http://gerrit.cloudera.org:8080/#/c/10291/1//COMMIT_MSG@7
PS1, Line 7: IMPALA-6948,IMPALA-6962: add end-to-end tests
> best practice is to spell out IMPALA-6962 so various tools can find it
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 04 May 2018 05:15:36 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6948,IMPALA-6962: add end-to-end tests

2018-05-03 Thread Vuk Ercegovac (Code Review) 
Hello Alex Behm,

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

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

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

Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests
..

IMPALA-6948,IMPALA-6962: add end-to-end tests

Adds end-to-end tests to validate that following
various metadata operations, the catalog state
in catalogd and impalads is the same.

For IMPALA-6962, catalogd process restart for tests
is fixed.

Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
---
M tests/common/impala_cluster.py
M tests/common/impala_service.py
A tests/custom_cluster/test_metadata_replicas.py
M tests/metadata/test_hms_integration.py
A tests/util/hive_utils.py
5 files changed, 254 insertions(+), 68 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5
Gerrit-Change-Number: 10291
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Vuk Ercegovac