verifiers.test_verify_metrics.TestValidateMetrics.test_metrics_are_zero
fails in ASAN tests. This is because a client session opened by
test_query_cancel_load_tables is not cleanly closed.
This patch fix the issue by creating the Impala client within the run
method of web_pages_util.py using with-as statement.
This patch also attempt to fix flakiness in test_query_expiration by
validating that time_limit_expire_handle is actually closed in
Coordinator log (IMPALA-13444).
Testing:
- Pass test_web_pages.py test in ASAN build.
- Loop and pass test_query_expiration 50x in ASAN build.
Change-Id: I6fcf13902478535f6fa15f267edc83891057993c
Reviewed-on: http://gerrit.cloudera.org:8080/23302
Reviewed-by: Daniel Becker <daniel.becker@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
This patch replace create_beeswax_client() reference to
create_hs2_client() or vector-based client creation to prepare towards
hs2 test migration.
test_session_expiration_with_queued_query is changed to use impala.dbapi
directly from Impyla due to limitation in ImpylaHS2Connection.
TestAdmissionControllerRawHS2 is migrated to use hs2 as default test
protocol.
Modify test_query_expiration.py to set query option through client
instead of SET query. test_query_expiration is slightly modified due to
behavior difference in hs2 ImpylaHS2Connection.
Remove remaining reference to BeeswaxConnection.QueryState.
Fixed a bug in ImpylaHS2Connection.wait_for_finished_timeout().
Fix some easy flake8 issues caught thorugh this command:
git show HEAD --name-only | grep '^tests.*py' \
| xargs -I {} impala-flake8 {} \
| grep -e U100 -e E111 -e E301 -e E302 -e E303 -e F...
Testing:
- Pass exhaustive tests.
Change-Id: I1d84251835d458cc87fb8fedfc20ee15aae18d51
Reviewed-on: http://gerrit.cloudera.org:8080/22700
Reviewed-by: Riza Suminto <riza.suminto@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
We have plenty of custom_cluster tests that assert against content of
Impala daemon log files while the process is still running using
assert_log_contains() and it's wrappers. The method specifically mention
about disabling glog buffering ('-logbuflevel=-1'), but not all
custom_cluster tests do that. This often result in flaky test that hard
to triage and often neglected if it does not frequently run in core
exploration.
This patch adds boolean param 'disable_log_buffering' into
CustomClusterTestSuite.with_args for test to declare intention to
inspect log files in live minicluster. If it is True, start minicluster
with '-logbuflevel=-1' for all daemons. If it is False, log WARNING on
any calls to assert_log_contains().
There are several complex custom_cluster tests that left unchanged and
print out such WARNING logs, such as:
- TestQueryLive
- TestQueryLogTableBeeswax
- TestQueryLogOtherTable
- TestQueryLogTableHS2
- TestQueryLogTableAll
- TestQueryLogTableBufferPool
- TestStatestoreRpcErrors
- TestWorkloadManagementInitWait
- TestWorkloadManagementSQLDetails
This patch also fixed some small flake8 issues on modified tests.
There is a flakiness sign at test_query_live.py where test query is
submitted to coordinator and fail because sys.impala_query_live table
has not exist yet from coordinator's perspective. This patch modify
test_query_live.py to wait for few seconds until sys.impala_query_live
is queryable.
Testing:
- Pass custom_cluster tests in exhaustive exploration.
Change-Id: I56fb1746b8f3cea9f3db3514a86a526dffb44a61
Reviewed-on: http://gerrit.cloudera.org:8080/22015
Reviewed-by: Jason Fehr <jfehr@cloudera.com>
Reviewed-by: Michael Smith <michael.smith@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
IMPALA-12602 introduced registering idle queries with a session so that
we can expire queries while still making their status available, and
clean up the idle query status when sessions are closed. That happens in
ImpalaServer::ExpireQueries, where it needs to acquire the
query_expiration_lock_ then a session_state->lock.
However that violated the lock order documented in impala-server.h, and
led to a deadlock when a query is expired at the same time another query
is registering expiration timers (which follows the documented order).
When the deadlock occurs, SetQueryInFlight holds a session_state->lock
and tries to acquire query_expiration_lock_, while ExpireQueries holds
the query_expiration_lock_ and tries to acquire session_state->lock.
The prior order between query_expiration_lock_ and session_state->lock
was largely arbitrary. query_expiration_lock_ operations don't
inherently require holding the session_state->lock. However expiration
operations work on a queue of ClientRequestStates that map to different
session states, so when we need to operate on a session state as part of
expiration we pretty much have to take query_expiration_lock_ first.
Updates lock order to take query_expiration_lock_ before
session_state->lock, and modifies SetQueryInFlight to release the
session_state->lock before registering expiration timers. The expiration
timers aren't related to the session, and query lifetime is maintained
by the QueryHandle reference.
Adds a custom cluster test that uses debug actions to reproduce the
deadlock scenario.
Change-Id: I6fce4103f6eeb7e9a4320ba1da817cab81071ba3
Reviewed-on: http://gerrit.cloudera.org:8080/21699
Reviewed-by: Michael Smith <michael.smith@cloudera.com>
Tested-by: Michael Smith <michael.smith@cloudera.com>
Queries cancelled due to idle_query_timeout/QUERY_TIMEOUT_S are now also
Unregistered to free any remaining memory, as you cannot fetch results
from a cancelled query.
Adds a new structure - idle_query_statuses_ - to retain Status messages
for queries closed this way so that we can continue to return a clear
error message if the client returns and requests query status or
attempts to fetch results. This structure must be global because HS2
server can only identify a session ID from a query handle, and the query
handle no longer exists. SessionState tracks queries added to
idle_query_statuses_ so they can be cleared when the session is closed.
Also ensures MarkInactive is called in ClientRequestState when Wait()
completes. Previously WaitInternal would only MarkInactive on success,
leaving any failed requests in an active state until explicitly closed
or the session ended.
The beeswax get_log RPC will not return the preserved error message or
any warnings for these queries. It's also possible the summary and
profile are rotated out of query log as the query is no longer inflight.
This is an acceptable outcome as a client will likely not look for a
log/summary/profile after it times out.
Testing:
- updates test_query_expiration to verify number of waiting queries is
only non-zero for queries cancelled by EXEC_TIME_LIMIT_S and not yet
closed as an idle query
- modified test_retry_query_timeout to use exec_time_limit_s because
queries closed by idle_timeout_s don't work with get_exec_summary
Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0
Reviewed-on: http://gerrit.cloudera.org:8080/21074
Reviewed-by: Michael Smith <michael.smith@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Python 3 changes list operators such as range, map, and filter
to be lazy. Some code that expects the list operators to happen
immediately will fail. e.g.
Python 2:
range(0,5) == [0,1,2,3,4]
True
Python 3:
range(0,5) == [0,1,2,3,4]
False
The fix is to wrap locations with list(). i.e.
Python 3:
list(range(0,5)) == [0,1,2,3,4]
True
Since the base operators are now lazy, Python 3 also removes the
old lazy versions (e.g. xrange, ifilter, izip, etc). This uses
future's builtins package to convert the code to the Python 3
behavior (i.e. xrange -> future's builtins.range).
Most of the changes were done via these futurize fixes:
- libfuturize.fixes.fix_xrange_with_import
- lib2to3.fixes.fix_map
- lib2to3.fixes.fix_filter
This eliminates the pylint warnings:
- xrange-builtin
- range-builtin-not-iterating
- map-builtin-not-iterating
- zip-builtin-not-iterating
- filter-builtin-not-iterating
- reduce-builtin
- deprecated-itertools-function
Testing:
- Ran core job
Change-Id: Ic7c082711f8eff451a1b5c085e97461c327edb5f
Reviewed-on: http://gerrit.cloudera.org:8080/19589
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Joe McDonnell <joemcdonnell@cloudera.com>
This takes steps to make Python 2 behave like Python 3 as
a way to flush out issues with running on Python 3. Specifically,
it handles two main differences:
1. Python 3 requires absolute imports within packages. This
can be emulated via "from __future__ import absolute_import"
2. Python 3 changed division to "true" division that doesn't
round to an integer. This can be emulated via
"from __future__ import division"
This changes all Python files to add imports for absolute_import
and division. For completeness, this also includes print_function in the
import.
I scrutinized each old-division location and converted some locations
to use the integer division '//' operator if it needed an integer
result (e.g. for indices, counts of records, etc). Some code was also using
relative imports and needed to be adjusted to handle absolute_import.
This fixes all Pylint warnings about no-absolute-import and old-division,
and these warnings are now banned.
Testing:
- Ran core tests
Change-Id: Idb0fcbd11f3e8791f5951c4944be44fb580e576b
Reviewed-on: http://gerrit.cloudera.org:8080/19588
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Joe McDonnell <joemcdonnell@cloudera.com>
Python 3 does not support this old except syntax:
except Exception, e:
Instead, it needs to be:
except Exception as e:
This uses impala-futurize to fix all locations of
the old syntax.
Testing:
- The check-python-syntax.sh no longer shows errors
for except syntax.
Change-Id: I1737281a61fa159c8d91b7d4eea593177c0bd6c9
Reviewed-on: http://gerrit.cloudera.org:8080/19551
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Reviewed-by: Michael Smith <michael.smith@cloudera.com>
Tested-by: Michael Smith <michael.smith@cloudera.com>
There was a recent flake where the number of in-flight queries
that were executing differed from the expected number.
The reason for the false negative is that one of the queries
expired before the check for in-flight queries: it took too long
to issue the queries.
This change modifies the timeout for the expired query to not expire
so quickly. Additional logging is added to the check for in-flight
queries so that we can distinguish the case of too few queries vs.
queries that have the wrong state.
Change-Id: I01a8762d28ad920b9ec8a0b1b82469618c66768f
Reviewed-on: http://gerrit.cloudera.org:8080/10602
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Recent tests ran into flakiness when testing query expiration.
This change makes two changes:
1) query state is retrieved earlier; a flaky test skipped
the expected state.
2) bump the timing; a flaky test had queries expire before
it could check them
Change-Id: I93f4ec450fc7e5a685c135b444e90d37e632831d
Reviewed-on: http://gerrit.cloudera.org:8080/10279
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
The test_query_expiration test assumes that a metric and
the query state are maintained atomically. Since they're
not, occasionaly flakes (false negatives) occur.
The fix in this patch is to loop until the expected state
is seen. If the expected state is not seen with a given number
of iterations, the test fails. These tests depend on timing so
if this validation takes too long, the test will also fail.
Such looping is used in the two places where its assumed that the
client state and metrics are maintained atomically.
Change-Id: I7aabed87d84d5cfd8078cc6c39df48e22ff30afc
Reviewed-on: http://gerrit.cloudera.org:8080/9538
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
This is similar to the QUERY_TIMEOUT_S option and shares most of the
implementation. The difference is that the timeout doesn't reset at
any point.
The time limit is measured from the start of query execution,
after the query is admitted, so planning, scheduling and
time spent in admission control is not counted towards the time limit.
Also fix validation of the related QUERY_TIMEOUT_S option, which
previously could ignore invalid input.
Testing:
Added tests for various permutations:
* With and without query_timeout_s set
* With and without result fetching keeping the query active
Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Reviewed-on: http://gerrit.cloudera.org:8080/9227
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
After IMPALA-4735 has been fixed, this suffix is no longer needed to
make test names prefix-free.
Change-Id: Ie63d145c94c02ec67e81d0c51a33d20685fba73e
Reviewed-on: http://gerrit.cloudera.org:8080/5886
Reviewed-by: Jim Apple <jbapple-impala@apache.org>
Tested-by: Impala Public Jenkins
The last active time in impala-server.cc#L1806 is in milliseconds, but
the TimestampValue c'tor expects seconds. This change also renames some
variables to make their meaning more explicit, aiming to prevent similar
bugs in the future.
This change also fixes a bug that occurred when during startup of the
local minicluster the operating system PIDs would wrap around. This way
the first impalad would not be the one with the smallest PID and
ImpalaCluster.get_first_impalad() would return the wrong one.
I ran git-clang-format on the change.
Change-Id: I283564c8d8e145d44d9493f4201555d3a1087edf
Reviewed-on: http://gerrit.cloudera.org:8080/5546
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Reviewed-by: Marcel Kornacker <marcel@cloudera.com>
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
For files that have a Cloudera copyright (and no other copyright
notice), make changes to follow the ASF source file header policy here:
http://www.apache.org/legal/src-headers.html#headers
Specifically:
1) Remove the Cloudera copyright.
2) Modify NOTICE.txt according to
http://www.apache.org/legal/src-headers.html#notice
to follow that format and add a line for Cloudera.
3) Replace or add the existing ASF license text with the one given
on the website.
Much of this change was automatically generated via:
git grep -li 'Copyright.*Cloudera' > modified_files.txt
cat modified_files.txt | xargs perl -n -i -e 'print unless m#Copyright.*Cloudera#i;'
cat modified_files_txt | xargs fix_apache_license.py [1]
Some manual fixups were performed following those steps, especially when
license text was completely missing from the file.
[1] https://gist.github.com/anonymous/ff71292094362fc5c594 with minor
modification to ORIG_LICENSE to match Impala's license text.
Change-Id: I2e0bd8420945b953e1b806041bea4d72a3943d86
Reviewed-on: http://gerrit.cloudera.org:8080/3779
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Internal Jenkins
Many of our test scripts have import statements that look like
"from xxx import *". It is a good practice to explicitly name what
needs to be imported. This commit implements this practice. Also,
unused import statements are removed.
Change-Id: I6a33bb66552ae657d1725f765842f648faeb26a8
Reviewed-on: http://gerrit.cloudera.org:8080/3444
Reviewed-by: Michael Brown <mikeb@cloudera.com>
Tested-by: Internal Jenkins
In order to make the query life-cycle clearer to users, added
a new section to the /queries webui page for queries that are
'waiting', not actively running either due to an error or to
returning all of their results, but that have not been closed so
they are still using resources.
This section is marked 'waiting to be closed' to indicate that they
still need to be closed even though they are not actively running.
These queries previously would have appeared in the 'in flight' list.
There is a tooltip with a full explanation.
The 'in_flight_queries' json endpoint was left as is, so that CM
will continue to work as expected, and filtering queries for the
different lists is done in the html template.
This was tested manually.
Change-Id: I47d0b642ecb573fefbbf337b8c8f2c479b0d49b2
Reviewed-on: http://gerrit.cloudera.org:8080/2625
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Internal Jenkins
Many python files had a hashbang and the executable bit set though
they were not intended to be run a standalone script. That makes
determining which python files are actually scripts very difficult.
A future patch will update the hashbang in real python scripts so they
use $IMPALA_HOME/bin/impala-python.
Change-Id: I04eafdc73201feefe65b85817a00474e182ec2ba
Reviewed-on: http://gerrit.cloudera.org:8080/599
Reviewed-by: Casey Ching <casey@cloudera.com>
Reviewed-by: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Tested-by: Internal Jenkins
Query expiration should take the minimum of --idle_query_timeout and
QUERY_TIMEOUT_S as the timeout for each query, but if either is 0 it
should be ignored.
This patch fixes that behaviour - previously if --idle_query_timeout was
0 it was taken as the timeout (and queries with 0 timeout are considered
not to be eligible for expiration).
There's a new test which confirms the fix.
Change-Id: Ieb960866b818a630f3d3c427d9454ca2ffd3f87c
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/3913
Reviewed-by: Henry Robinson <henry@cloudera.com>
Reviewed-by: Daniel Hecht <dhecht@cloudera.com>
Tested-by: jenkins
(cherry picked from commit 35f8ef45ef8774a6a70d3b35fc20ea14ab7c0d49)
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/3942
SET QUERY_TIMEOUT_S=30 for a 30s idle query timeout. The maximum and
default query timeouts are both controlled by --idle_query_timeout, if > 0.
Change-Id: I2d2a7fd982cbd7ec34c0496dffa146d8a10adabb
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/3617
Reviewed-by: Henry Robinson <henry@cloudera.com>
Tested-by: jenkins
(cherry picked from commit ba5a619660a826e90d6e43c803e50150cf90e694)
Reviewed-on: http://gerrit.sjc.cloudera.com:8080/3687
A crucial comparison was between time values with different units.
Tests didn't catch this because they only confirmed that sessions were
timed out within the correct time, not that they were *not* timed out
early.
Change-Id: Ia8c57d3d70e4702996d0225b167142b7bf88d236
Reviewed-on: http://gerrit.ent.cloudera.com:8080/1926
Tested-by: jenkins
Reviewed-by: Henry Robinson <henry@cloudera.com>
This is the first step in cleaning up the test logging. It provides a common connection
interface that provides tracing around all operations. When a test fails the output will
be executable SQL. It also logs actions such as when a connection is opened, close, or
when an operation is cancelled. Currently only beeswax connections are supported, but
I have a seperate patch that adds support for executing using HS2 as well as Beeswax.
Example of new logging:
-- connecting to: localhost:21000
-- executing against localhost:21000
use functional;
SET disable_codegen=False;
SET abort_on_error=1;
SET batch_size=0;
SET num_nodes=0;
-- executing against localhost:21000
select a.timestamp_col from alltypessmall a inner join alltypessmall b on
(a.timestamp_col = b.timestamp_col)
where a.year=2009 and a.month=1 and b.year=2009 and b.month=1;
-- closing connection to: localhost:21000
Change-Id: Iedc7d4d3a84bfeff6cc1daae6ed1ca97613d7700
Reviewed-on: http://gerrit.ent.cloudera.com:8080/1133
Tested-by: jenkins
Reviewed-by: Lenni Kuff <lskuff@cloudera.com>
Test suites that derive from common.CustomClusterTestSuite have a brand
new cluster for every tests case, which they can configure as they wish
with custom arguments using the @with_args() decorator.
A future improvement is to optionally only have one cluster per test
suite, to allow multiple tests to run more quickly if they share
configuration options.
Change-Id: I6abd5740e644996d7ca2800edf4ff11b839d1bc4
Reviewed-on: http://gerrit.ent.cloudera.com:8080/882
Reviewed-by: Henry Robinson <henry@cloudera.com>
Tested-by: jenkins
