CFB mode is a stream cipher and is secure when used with a different nonce/IV
for every message. However it can be a performance bottleneck.
CTR mode is also stream cipher and is secure, 4~6x faster than CFB mode in
OpenSSL. AES-CTR+SHA256 is about 40~70% faster than AES-CFB+SHA256.
CTR mode is used if OpenSSL version>=1.0.1 at runtime, otherwise
fall back to using CFB mode.
Testing:
run runtime tmp-file-mgr-test, openssl-util-test, buffer-pool-test and
buffered-tuple-stream-test
The ut case openssl-util-test.EncryptInPlace tests encryption in both modes.
Change-Id: I9debc240615dd8cdbf00ec8730cff62ffef52aff
Reviewed-on: http://gerrit.cloudera.org:8080/8861
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
This patch maps a signed integer logical type in parquet to a supported
Impala column type. This change introduces the following mapping -
INT_8 -> TINYINT
INT_16 -> SMALLINT
INT_32 -> INT
INT_64 -> BIGINT
Also, added a parquet file with the following schema for testing -
schema {
optional int32 id;
optional int32 tinyint_col (INT_8);
optional int32 smallint_col (INT_16);
optional int32 int_col;
optional int64 bigint_col;
}
Change-Id: I47a8371858c9597c6a440808cf6f933532468927
Reviewed-on: http://gerrit.cloudera.org:8080/8548
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Reviewed-by: Tianyi Wang <twang@cloudera.com>
Tested-by: Impala Public Jenkins
Without this patch, redaction is applied to every field in the
runtime profile. This approach has an undesired side effect when
Kerberos auth + email redaction is in place.
Since the redaction applies to every field, even principals
(from Connected/Delegated User fields) are redacted, as the Kerberos
principal format generally pattern matches with an email redactor
template.
This is particularly problematic for monitoring tools that consume
runtime profiles and use these fields to group the queries by user.
This patch fixes the problem by redacting only the following sensitive
fields.
- Query Statement
- Error logs (since they can contain column references etc.)
- Query Status
- Query Plan
Other fields in the runtime profile are left unredacted.
Change-Id: Iae3b6726009bf458a7ec73131e5d659b12ab73cf
Reviewed-on: http://gerrit.cloudera.org:8080/8934
Reviewed-by: Bharath Vissapragada <bharathv@cloudera.com>
Tested-by: Impala Public Jenkins
This commit makes idle_session_timeout a query option.
idle_session_timeout currently can be set as a command line
option, which will be the default timeout for sessions.
HS2 sessions can override it with a smaller value by setting
it in the configuration overlay of HS2 OpenSession().
However, we can't override idle_session_timeout for JDBC/ODBC
connections, because we cannot put this in the connection string.
This commit is a workaround for this problem, it allows JDBC/ODBC
connections to set the session timeout as a query option
with the SET statement.
After this commit, the session timeout can be overridden to
any value, i.e. the command line flag idle_session_timeout
doesn't limit this option anymore.
I created an automated test case in JdbcTest.java based on
test_hs2.py::test_concurrent_session_mixed_idle_timeout. I also
extended the test_session_expiration and test_set_and_unset
test suites.
Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e
Reviewed-on: http://gerrit.cloudera.org:8080/8490
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Currently DictDecoder class and DictEncoder class uses std::vector
to store the tables mapping codeword to value and vice-versa. It is
hard to detect the memory usage by these tables when they becomes
very large, since this memory is not accounted by Impala's memory
mangement infrastructure.
This patch uses the memory tracker of HdfsScanner to track the memory used
by dictionary in DictDecoder class. Similary it uses memory tracker of
HdfsTableSink to track the memory used by dictionary in DictEncoder class.
Memory for the dictionary, stored as std::vector is still allocated
from std:allocator but the amount allocated is accounted by
introducing a counter which is incremented and decremented as the
memory is consumed and released by vector.
Testing
-------
Ran all the backend and end-end tests with no failures.
Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299
Reviewed-on: http://gerrit.cloudera.org:8080/8034
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Avoid the circular dependency between ReservationTracker::lock_ and
MemTracker::child_trackers_lock_ by not acquiring
ReservationTracker::lock_ in GetReservation(), where an atomic
operation is sufficient.
Testing:
Added a unit test that reproed the deadlock.
Change-Id: Id7adbe961a925075422c685690dd3d1609779ced
Reviewed-on: http://gerrit.cloudera.org:8080/8933
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Currently, all HdfsFileHandles are owned and constructed
by the file handle cache. When the file handle cache
is disabled or the file handle is not eligible for
caching, the HdfsFileHandle is stored exclusively in
ScanRange::exclusive_hdfs_fh_, but the HdfsFileHandle still
comes from the file handle cache. It is created via a call to
DiskIoMgr::GetCachedHdfsFileHandle() with 'require_new_handle'
set to true and destroyed via
DiskIoMgr::ReleaseCachedHdfsFileHandle() with 'destroy_handle'
set to true.
Recent testing has revealed that the lock on the file handle
cache is a bottleneck for workloads with many small remote
files. There is no benefit to storing these exclusive file
handles in the file handle cache, as they do not participate
in the caching.
This change introduces DiskIoMgr::GetExclusiveHdfsFileHandle()
and DiskIoMgr::ReleaseExclusiveHdfsFileHandle(). These are
equivalent to the Get/ReleaseCachedHdfsFileHandle() calls, except
they bypass the file handle cache and create/destroy the
file handle directly. ScanRange::Open()/Close(), which
populates and frees ScanRange::exclusive_hdfs_fh_, now uses
these new calls rather than accessing the file handle cache.
This avoids the locking entirely, solving the bottleneck.
To draw a distinction between the two codepaths, HdfsFileHandle
is now an abstract class with two subclasses:
- CachedHdfsFileHandles cover all handles that live in file handle
cache. Get/ReleaseCachedHdfsFileHandle() use this subclass.
- ExclusiveHdfsFileHandles cover all cases where a file handle
does not come from the cache. The new
Get/ReleaseExclusiveHdfsFileHandle() use this subclass.
Separately, testing revealed that increasing the number of
partitions for the file handle cache also fixes the contention
problem. This changes the file handle cache to make the number
of partitions configurable via startup parameter
num_file_handle_cache_partitions. This allows mitigation of
future bottlenecks without a patch.
Change-Id: I4ab52b0884a909a4faeb6692f32d45878ea2838f
Reviewed-on: http://gerrit.cloudera.org:8080/8945
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Impala Public Jenkins
When materialising a nested collection, has_template_tuple() should use
the template tuple for the collection, not the top-level tuple.
Testing:
Added tests based on nested-types-basic.test that operate on a simple
partitioned table. The tests reliably crashed Impala before the fix.
Change-Id: Ic808b824ce3b31af0539036d8ca23d17b18deab4
Reviewed-on: http://gerrit.cloudera.org:8080/8947
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Currently, when debug mode is enabled, any query using codegen can result
in an Impala daemon crash as it hits a DCHECK.
This patch ensures the DCHECK is hit only when specific condition is met to
avoid the crash. That condition here is to DCHECK only when 'emit_perf_map_'
evaluates to True ensuring 'perf_map_lock_' is not empty when asserted.
Change-Id: I93e2b1efb325100d01d398e68e789d87b877167e
Reviewed-on: http://gerrit.cloudera.org:8080/8923
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
The bug is that ConvertToInt128(), by design, only sets 'overflow' if an
overflow occurs. This means that caller needs to initialise the overflow
variable to false, otherwise the value when overflow occurs is
undefined.
Testing:
Reprod the expr-test failure under ASAN, confirmed that it passed after
this fix.
Change-Id: Ifd7ac691155442ba7cba71dd3647208b7c1c0bf9
Reviewed-on: http://gerrit.cloudera.org:8080/8929
Reviewed-by: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Reviewed-by: Michael Brown <mikeb@cloudera.com>
Tested-by: Impala Public Jenkins
This patch fixes several issues related to the min/max aggregate
functions and their handling of 'nan' and 'inf':
- Previously, if 'inf' or '-inf' was the only value for the min/max
and codegen was being used, the result would be incorrect. This
occurred, for example in the case of 'inf' and 'min', because we
set an initial value of numeric_limits::max, which is less than
'inf', so the returned min was numeric_limits::max when it should be
'inf'. The fix is to set the initial value to
numeric_limits::infinity.
- Previously, if one of the values was 'nan', the result of min/max
was non-deterministic depending on the order the values were
evaluated in. This occurs because 'nan' < or > 'any value' is always
false, so if the first value added was 'nan', all other comparisons
would be false and 'nan' would be returned, whereas if the first
value wasn't 'nan' then the 'nan' wouldn't be returned. The fix is
to treat 'nan' specially and to always return 'nan' if there is a
single 'nan' value.
Testing:
- Added e2e tests for both scenarios, as well as adding a little extra
nan/inf coverage for other aggregate functions.
Change-Id: Ia1e206105937ce5afc75ca5044597d39b3dc6a81
Reviewed-on: http://gerrit.cloudera.org:8080/8854
Reviewed-by: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Currently, the RPC layer accesses many gflags directly to take
certain decisions, eg. whether to turn on encryption,
authentication, etc.
Since the RPC layer is to be used more like a library, these should
be configurable options that are passed to the Messenger
(which is the API endpoint for the application using the RPC layer),
instead of the RPC layer itself directly accessing these flags.
This patch converts the following flags to Messenger options and moves
the flag definitions to server_base.cc which is the "application" in
Kudu that uses the Messenger:
FLAGS_rpc_default_keepalive_time_ms
FLAGS_rpc_negotiation_timeout_ms
FLAGS_rpc_authentication
FLAGS_rpc_encryption
FLAGS_rpc_tls_ciphers
FLAGS_rpc_tls_min_protocol
FLAGS_rpc_certificate_file
FLAGS_rpc_private_key_file
FLAGS_rpc_ca_certificate_file
FLAGS_rpc_private_key_password_cmd
FLAGS_keytab_file
Most of the remaining flags are test or benchmark related flags. There
may be a few more flags that can be moved out and converted to options,
but we can leave that as future work if we decide to move them.
In addition to the cherry-pick above, this change also updates Impala
code to pass the key_tab file to InitKerberosForServer() which was changed
by this Kudu patch.
Change-Id: Ia21814ffb6e283c2791985b089878b579905f0ba
Reviewed-on: http://gerrit.cloudera.org:8080/8789
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <danburkert@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/8878
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Impala Public Jenkins
This change adds a new flag FLAGS_tcmalloc_max_total_thread_cache_bytes
which specifies the maximum size in bytes which the total of all TCMalloc
thread caches can grow to. By default, it's set to 0 and the default value
in TCMalloc library is used. This change also always enables "aggressive
decommit" feature in TCMalloc instead of just validating that it's enabled
in the code by default.
Testing done: core build;
test_tpch.py with FLAGS_tcmalloc_max_total_thread_cache_bytes set.
Change-Id: Ib968ee7d20458143ef6ac14ad3ac2c4d84d31dc5
Reviewed-on: http://gerrit.cloudera.org:8080/8906
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
This change makes sure backend connections with KRPC are always
kept alive by disabling the idle connection detection logic.
Idle connections all tend to be closed and re-opened around the
same time, which may easily lead to negotiation timeouts. Until
KUDU-279 is fixed, closing idle connections is also racy and leads
to query failures.
This disablement was validated with Kudu's rpc-test.
Change-Id: I0871dd9c9bbe455466b9b2d2a2bbedec79cf0775
Reviewed-on: http://gerrit.cloudera.org:8080/8910
Reviewed-by: Michael Ho <kwho@cloudera.com>
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Impala Public Jenkins
Currently, if an error is encountered during the creation of a
handcrafted codegen method, then the resulting IR is left in an
incomplete state. This patch ensures that all such IRs are cleaned up
(method is deleted from the module) before the llvm module is finalized.
Testing:
- added a backend test to exercise the added code path.
- tested manually by executing the following query:
select * from charTable A, charTable B
where A.charColumn = B.charColumn and A.charColumn = 'foo';
and looking at the logs to verify that 'InsertRuntimeFilters' and
'FilterContextInsert' methods have been removed.
Change-Id: If975cfb3906482b36dd6ede32ca81de6fcee1d7f
Reviewed-on: http://gerrit.cloudera.org:8080/8541
Reviewed-by: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Tested-by: Impala Public Jenkins
Currently, a server connection being idle for more than
FLAGS_rpc_default_keepalive_time_ms ms will be closed.
However, certain services (e.g. Impala) using KRPC may want to
keep the idle connections alive for various reasons (e.g. sheer
number of connections to re-establish, negotiation overhead
in a secure cluster). To avoid idle connection from being
closed, one currently have to set FLAGS_rpc_default_keepalive_time_ms
to a very large value.
This change implements a cleaner solution by disabling idle
connection scanning if FLAGS_rpc_default_keepalive_time_ms is
set to any negative value. This avoids the unnecessary
overhead of scanning for idle server connections and alleviates
the user from having to pick a random large number to make sure
the connection is always kept alive.
Change-Id: I6161b9e753f05620784565a417d248acf8e7050a
Reviewed-on: http://gerrit.cloudera.org:8080/8831
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/8909
Reviewed-by: Lars Volker <lv@cloudera.com>
Tested-by: Impala Public Jenkins
In order to compute the modulo of two decimals, we need to bring the
underlying datatype to the same scale first. It turns out we could
overflow when scaling up one of the values.
In this patch we fix the problem by using a larger data type when we
detect that the scaled up value will not fit into the original data
type.
Testing:
- Added some expr tests that reproduce the issue.
Change-Id: I27c7f25f68353c19c315e1639311ec06b2dea686
Reviewed-on: http://gerrit.cloudera.org:8080/8833
Reviewed-by: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Tested-by: Impala Public Jenkins
In this patch we implement rounding when casting string to decimal if
DECIMAL_V2 is enabled. The backend method that parses strings and
converts them to decimals is refactored to make it easier to understand.
Testing:
- Added some BE tests.
Change-Id: Icd8b92727fb384e6ff2d145e4aab7ae5d27db26d
Reviewed-on: http://gerrit.cloudera.org:8080/8774
Reviewed-by: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Tested-by: Impala Public Jenkins
This adds a workaround for an issue reported on the user mailing list.
Some systems are configured such that the auth_to_local mapping provided
by the krb5 library doesn't work properly for service accounts.
This patch adds a new configuration which allows Kudu to disregard the
system auth_to_local rules and instead just map kerberos principals to
their first component, which is typically the username.
Change-Id: I2e893493f52965ea54d2ceaac83d375285b49486
Reviewed-on: http://gerrit.cloudera.org:8080/8373
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Reviewed-by: Dan Burkert <danburkert@apache.org>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/8875
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Impala Public Jenkins
In the commit 4feb4f3a, the third party library pcg-cpp was excluded
from the clang-tidy check. It could make unexpected side effect, so
fixing some warnings from clang-tidy is better than avoidance of the
check.
Change-Id: I591d30373cb13f0eb89afbe16d81b1d3fb783365
Reviewed-on: http://gerrit.cloudera.org:8080/8829
Reviewed-by: Jim Apple <jbapple-impala@apache.org>
Tested-by: Impala Public Jenkins
This commit introduces the ThreadDebugInfo class which can
hold information about the current thread that can be useful
in debug sessions. It needs to be allocated on the stack of
each thread in order to include it to minidumps.
Currently a ThreadDebugInfo object is created in
Thread::SuperviseThread. This object is available in all
the child stack frames through the global function
GetThreadDebugInfo().
ThreadDebugInfo has members for the thread name and instance
id. These are fixed size char buffers.
If you have a core dump written by Impala, you can locate the
ThreadDebugInfo for the current thread through the global
pointer impala::thread_debug_info.
In a core file that has been created from a minidump, we need
to select the stack frame that allocated the ThreadDebugInfo
object in order to inspect it. It is currently allocated in
Thread::SuperviseThread().
We can use printf in gdb to print the members, e.g.:
printf "%s\n" thread_debug_info.instance_id
Currently the thread name and instance id is stored.
I created some tests in thread-debug-info-test.cc
Change-Id: I566f7f1db5117c498e86e0bd05b33bdcff624609
Reviewed-on: http://gerrit.cloudera.org:8080/8621
Reviewed-by: Lars Volker <lv@cloudera.com>
Tested-by: Impala Public Jenkins
Modifies COMPUTE STATS TABLESAMPLE to use the new SAMPLED_NDV()
function.
Testing:
- modified/improved existing functional tests
- core/hdfs run passed
Change-Id: I6ec0831f77698695975e45ec0bc0364c765d819b
Reviewed-on: http://gerrit.cloudera.org:8080/8840
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
We saw some failures on the exhaustive release build because the
compiler assumed that the pointer to the intermediate struct that is
used for computing decimal average was aligned.
To fix the problem, we mark the struct with a "packed" attribute so
that the compiler does not expect it to be aligned.
Testing:
- Ran the failing test locally on an release build and it passed.
Change-Id: Id25ec6e20dde3f50fb37a22135b355ad251809e0
Reviewed-on: http://gerrit.cloudera.org:8080/8836
Reviewed-by: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Tested-by: Impala Public Jenkins
The current default for krpc_port is 29000, which conflicts
with Sentry's WebUI. This changes the default to 27000,
which is currently free.
Core tests pass with this change.
Change-Id: Iaf5ccedfd9bc1eff9786e4b019c1bb68bf757300
Reviewed-on: http://gerrit.cloudera.org:8080/8841
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Impala Public Jenkins
This patch adds the following details to the error message encountered
on failure to get minimum memory reservation:
- which ReservationTracker hit its limit
- top 5 admitted queries that are consuming the most memory under the
ReservationTracker that hit its limit
Testing:
- added tests to reservation-tracker-test.cc that verify the error
message returned for different cases.
- tested "initial reservation failed" condition manually to verify
the error message returned.
Change-Id: Ic4675fe923b33fdc4ddefd1872e6d6b803993d74
Reviewed-on: http://gerrit.cloudera.org:8080/8781
Reviewed-by: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Tested-by: Impala Public Jenkins
Testing under ASAN:
- reproduced locally
- does not reproduce after fix
- locally ran test_aggregation.py which passed
Change-Id: Ia695201e61d8afc23636f826264635c85d3a228a
Reviewed-on: http://gerrit.cloudera.org:8080/8838
Tested-by: Impala Public Jenkins
Reviewed-by: Jim Apple <jbapple-impala@apache.org>
Currently implementation of rand/random built-in functions
use rand_r of C library. We recognized its randomness was poor.
pcg32 of third party library shows better randomness than rand_r.
Testing:
Revise unit test in expr-test
Add E2E test to random.test
Change-Id: Idafdd5fe7502ff242c76a91a815c565146108684
Reviewed-on: http://gerrit.cloudera.org:8080/8355
Reviewed-by: Jim Apple <jbapple-impala@apache.org>
Tested-by: Impala Public Jenkins
This patch adds a new MemTracker under the Process MemTracker called
"TCMalloc Overhead" which accounts for different cache freelists
maintained by TCMalloc. This added accounting also helps bring down
the amount of untracked memory.
An example dump of the Process MemTracker now looks like:
Process: Limit=8.34 GB Total=119.10 MB Peak=119.10 MB
Buffer Pool: Free Buffers: Total=0
Buffer Pool: Clean Pages: Total=0
Buffer Pool: Unused Reservation: Total=0
TCMalloc Overhead: Total=11.42 MB
Untracked Memory: Total=107.69 MB
Testing:
Tested manually by checking the memz webpage.
Change-Id: I602e9d5e8e8d7470dcfe4addde3265057c16263a
Reviewed-on: http://gerrit.cloudera.org:8080/8782
Reviewed-by: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Tested-by: Impala Public Jenkins
Adds a new SAMPLED_NDV() aggregate function that is
intended to be used in COMPUTE STATS TABLESAMPLE.
This patch only adds the function itself. Integration
with COMPUTE STATS will come in a separate patch.
SAMPLED_NDV() estimates the number of distinct values (NDV)
based on a sample of data and the corresponding sampling rate.
The main idea is to collect several x/y data points where x is
the number of rows and y is the corresponding NDV estimate.
These data points are used to fit an objective function to the
data such that the true NDV can be extrapolated.
The aggregate function maintains a fixed number of HyperLogLog
intermediates to compute the x/y points.
Several objective functions are fit and the best-fit one is
used for extrapolation.
Adds the MPFIT C library to perform curve fitting:
https://www.physics.wisc.edu/~craigm/idl/cmpfit.html
The library is a C port from Fortran. Scipy uses the
Fortran version of the library for curve fitting.
Testing:
- added functional tests
- core/hdfs run passed
Change-Id: Ia51d56ee67ec6073e92f90bebb4005484138b820
Reviewed-on: http://gerrit.cloudera.org:8080/8569
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
Previously, DataStreamSender::FlushAndSendEos() makes some wrong
assumption about the format string of the error status returned
from DoTransmitDataRpc(). In particular, it assumes that the format
string has exactly one substitution argument. This is a pretty bad
assumption as new types of errors could be returned from
DoTransmitDataRpc() and they can take different numbers of
substitution arguments. Recent changes in the format string
TErrorCode::RPC_RECV_TIMEOUT exposed this bug.
This change fixes the problem by removing this bad usage of Status()
in data-stream-sender.cc
Change-Id: I1cc04db670069a7df484e2f2bafb93c5315b9c82
Reviewed-on: http://gerrit.cloudera.org:8080/8817
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
The error threshold in TimerCounterTest is 15ms, which is still not
enough in some rare cases. This patch increases it to 30ms.
Change-Id: Ifc038908857060ccbabfe30c46e72fd93907f412
Reviewed-on: http://gerrit.cloudera.org:8080/8670
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Adds a whitelist of LLVM CPU attributes that I know that we routinely
test Impala with. This excludes the problematic AVX512 attributes as
well as some other flags we don't test with - e.g. AMD-only
instructions, NVM-related instructions, etc. We're unlikely to get
significant benefit from these instruction set extensions without
explicitly using them via instrinsics.
Testing:
Ran core tests on a system with AVX512 support with a prototype patch
that disabled only the AVX512 flags. Added a backend test to make sure
that the whitelisting is working as expected.
Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a
Reviewed-on: http://gerrit.cloudera.org:8080/8802
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Previously, CodegenAnyVal used an LLVM function for floating point
comparison that considered 'nan' = 'nan' to be true. This is
inconsistent with the way we handle 'nan' in the non-codegen path,
where we consider 'nan' = 'nan' to be false, leading to inconsisent
results.
This patch fixes CodegenAnyVal to use an LLVM function for floating
point comparison that considers 'nan' = 'nan' to be false.
Testing:
- Added e2e tests for the two scenarios affected by this: CASE and
joins.
Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1
Reviewed-on: http://gerrit.cloudera.org:8080/8790
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
When subtracting two decimals, and one of them is large, we incorrectly
hit a DCHECK if the intermediate result was equal to 10^39-1, which is
MAX_UNSCALED_DECIMAL16. We fix the problem by changing the condition in
the DCHECK from "less than" to "less than or equal to".
Testing:
- Added expr tests that reproduce the issue.
Change-Id: I42d42ad85efe32b7a0db0d2353385939fee09934
Reviewed-on: http://gerrit.cloudera.org:8080/8796
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
There is not much benefit in printing the stack trace when
Thrift RPC hits an error. As long as we print enough info about
the error and identify the caller, that should be sufficient.
In fact, it has been observed that stack crawl caused unnecessary
CPU spikes in the past. This change replaces Status() with
Status::Expected() in DoRpc(), RetryRpc(), RetryRpcRecv() and
Coordinator::BackendState::Exec() to avoid unnecessary stack crawls.
Testing done: private core build. Verified error strings with
test_rpc_timeout.py and test_rpc_exception.py
Change-Id: Ia83294494442ef21f7934f92ba9112e80d81fa58
Reviewed-on: http://gerrit.cloudera.org:8080/8788
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
test_basic_filters has been occasionally failing due to a line missing
from a runtime profile for a particular query.
The problem is that the query returns all of its results before all of
its fragment instances are finished executing (due to a limit). Then,
when one fragment instance reports its status, the coordinator returns
to it a 'cancelled' status, causing all remaining instances for that
backend to be cancelled.
Sometimes this cancellation happens quickly enough that the relevant
fragment instances have not yet sent a status report when they are
cancelled. They will still send a report in finalize, but as the
coordinator only updates its runtime profile for 'ok' status reports,
not 'cancelled', the final runtime profile doesn't end up with any
data for those fragment instances, which means the test does not find
the line in the runtime profile its checking for.
The fix is to have the coordinator update its runtime profile with
every status report it recieves, regardless of error status.
Testing:
- Ran existing runtime profile tests, which rely on profile output,
in a loop.
- Manually tested some scenarios with failed queries and checked that
the new profile output is reasonable.
- Added a new e2e test that runs the affected query and checks for the
presence of info for all expected exec node in the profile. This
repros the underlying issue consistently.
Change-Id: I4f581c7c8039f02a33712515c5bffab942309bba
Reviewed-on: http://gerrit.cloudera.org:8080/8754
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Impala Public Jenkins
The KuduRPC subsystem uses kudu::ServicePool to service all incoming
RPCs. Since this lives inside the Kudu codebase, all instrumentation
of RPCs flowing through the KuduRPC subsystem is not visible to Impala,
as Kudu does not export this instrumentation, but only maintains it
internally within kudu::ServicePool.
In order to reliably view the instrumentation of all RPCs flowing through
KRPC, one option is to modify kudu::ServicePool to take their
instrumentation and display it on our webpages, which is very invasive.
A second option is to have a parallel implementation of kudu::ServicePool
which for all purposes behaves the same way, but instead has this extra
code that displays this instrumentation in our webpages.
This patch is Part 2 of the second option.
In Part 1, we simply copied the code from kudu::ServicePool into the Impala
namespace. In this patch, we rename the redundant kudu::ServicePool
(that was copied into be/src/rpc/) to ImpalaServicePool. We also expose
the instrumentaion of the ImpalaServicePool as JSON.
Now, the threads in use by the service pool will also be visible in
the /threadz page.
This patch however does not display instrumentation on the Webpages yet.
In a future patch, we will add that through the ImpalaServicePool and the
RpcMgr. Tracked by IMPALA-6269
This patch is partially based on an abandoned patch by Henry Robinson.
Change-Id: Ia3e0fa9ae22c4803b0cea736eb35e333e2671448
Reviewed-on: http://gerrit.cloudera.org:8080/8472
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Impala Public Jenkins
*** THIS PATCH IS NOT FOR REVIEW. IT IS JUST UPLOADED TO MAKE THE
REVIEW OF PART-2 EASIER IN GERRIT BY SHOWING ONLY THE DIFFS OF THAT
PATCH AGAINST THIS PATCH. MORE EXPLANATION BELOW***
The KuduRPC subsystem uses kudu::ServicePool to service all incoming
RPCs. Since this lives inside the Kudu codebase, all instrumentation
of RPCs flowing through the KuduRPC subsystem is not visible to Impala,
as Kudu does not export this instrumentation, but only maintains it
internally within kudu::ServicePool.
In order to reliably view the instrumentation of all RPCs flowing through
KRPC, one option is to modify kudu::ServicePool to take their
instrumentation and display it on our webpages, which is very invasive.
A second option is to have a parallel implementation of kudu::ServicePool
which for all purposes behaves the same way, but instead has this extra
code that displays this instrumentation in our webpages.
This patch is Part 1 of the second option. In this patch, we simply copy
the kudu::ServicePool code into be/src/rpc/.
In the next patch (Part 2), we rename it to ImpalaServicePool,
and we do the instrumentation such that it shows up on the
webpage.
We split it up this way into 2 parts so that it will be easy to review.
Change-Id: I5bb927d4726d4fe418251b9734a4e232810fef69
Reviewed-on: http://gerrit.cloudera.org:8080/8471
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Sailesh Mukil <sailesh@cloudera.com>
Previously, we implicitly create a local string object created from
the char* in argv[0] when calling InitAuth(). This string object goes
out of scope once InitAuth() returns but the pointer of this local
string's buffer is passed to the Sasl library which may reference
it after the local string has been deleted, leading to use-after-free.
This bug is exposed by recent change to enable Kerberos with KRPC as
we now always initialize Sasl even if Kerberos is not enabled.
This change fixes the problem above by making a copy of 'appname'
passed to InitAuth(). Also, the new code enforces that multiple
calls to InitAuth() must use the same 'appname' or it will fail.
Testing done: Verified rpc-mgr-test and thrift-server-test no longer
fail in ASAN build.
Change-Id: I1f29c2396df114264dfc23726b8ba778f50e12e9
Reviewed-on: http://gerrit.cloudera.org:8080/8777
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Reviewed-by: Lars Volker <lv@cloudera.com>
Tested-by: Impala Public Jenkins
This change moves the creation of the runtime profile from DataSink::Prepare()
to the ctor of DataSink derived classes. This makes sure that DataSink::Close()
and other functions can access the profile even if the DataSink fails to initialize.
Testing done: Added a test case which triggers failure in the initialization of output
expressions in a HdfsTableSink. Impalad crashed consistently without the fix.
Change-Id: I2a683000ef180027b929dbebe78bc2a530a4767e
Reviewed-on: http://gerrit.cloudera.org:8080/8770
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
IMPALA-3798 disabled per-scan filtering for sequence-
based scanners due to a race between runtime filter
arrival and header splits processing.
This commit enables per-scan filtering again for the
sequence based files. In HdfsScanNode::ProcessSplit()
we check if the current range is the header of a
sequence file. If so, and the filters reject the file,
the whole file skipped.
If it is not a sequence header, but the filters reject
the partition, we call RangeComplete() on the current
scan range.
Change-Id: I4b38c26bcbe67f83efcc65a1723d766626ae3d3e
Reviewed-on: http://gerrit.cloudera.org:8080/8684
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
The Kudu security library currently sources the kerberos principal
directly from FLAGS_principal. Since this is a library, we'd rather
move this to be an option that is passed through InitKerberosForServer().
Users of the security library may pass any principal that they want
to.
Testing: All current tests pass. Since this is a minor refactor, no new
tests are needed.
Change-Id: Idd16b3360d8d2df5a609eb897bb9810e662fc695
Reviewed-on: http://gerrit.cloudera.org:8080/8700
Reviewed-by: Dan Burkert <danburkert@apache.org>
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/8760
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Tim Armstrong <tarmstrong@cloudera.com>
Some tools use lineage graph logging to collect query metrics. Currently
only query hash is present in this log. Adding query id into it makes
such accounting easier.
Testing: The equality of query id in the query profile and lineage log
is checked in test_lineage.py. A test for TUniqueIdUtil is added to the
FE tests.
Change-Id: I4adbd02df37a234dbb79f58b7c46ca11a914229f
Reviewed-on: http://gerrit.cloudera.org:8080/8589
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
In Impala, we have FLAGS_principal and FLAGS_be_principal flags.
If only FLAGS_principal is set, we use it as both the internal
and external principals.
If both FLAGS_principal and FLAGS_be_principal are set, we use
FLAGS_be_principal as the internal principal and FLAGS_principal
as the external principal.
However, in Kudu, they only source the internal principal from
FLAGS_principal and aren't aware of a flag called FLAGS_be_principal.
So as of the time IMPALA-5129 went in, if FLAGS_be_principal is
explicitly set to something different from FLAGS_principal, we would
be using the external principal as the internal principal, which is
incorrect.
This is fixed on the Kudu side by the following commit:
d42c291646
This is now cherry-picked to Impala and we now use the new API to
fix this problem.
Testing: Made the MiniKdc explicitly set FLAGS_principal and
FLAGS_be_principal. Confirmed that the tests fail without this
patch and with FLAGS_be_principal set.
Change-Id: If5af4398467857da09878075439b6612a04d7a01
Reviewed-on: http://gerrit.cloudera.org:8080/8761
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
This patch just disables the failing test to unblock builds.
We will investigate in parallel the root cause for these failures
and post a real fix.
Change-Id: I6c750850ff916617a06e3cfac330072d8e2179e8
Reviewed-on: http://gerrit.cloudera.org:8080/8766
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
There are scenarios where HDFS file appends or HDFS file
overwrites can lead to HDFS disabling short circuit reads.
Since this can be a performance regression, this changes
the default value for max_cached_file_handles to 0 to
disable the file handle cache by default. This also changes
the default value for unused_file_handle_timeout_sec to 270.
If users enable the file handle cache, this setting will
prevent some of the scenarios that disable short circuit
reads.
Ran existing file handle cache tests to verify that there
is no impact.
Change-Id: Iea7f943f63b72b42286a9e8b9987308baa79d7b0
Reviewed-on: http://gerrit.cloudera.org:8080/8750
Reviewed-by: Joe McDonnell <joemcdonnell@cloudera.com>
Tested-by: Impala Public Jenkins
This change augments the message of TErrorCode::DATASTREAM_SENDER_TIMEOUT
to include the source address when KRPC is enabled. The source address is
not readily available in Thrift. The new message includes the destination
plan node id in case there are multiple exchange nodes in a fragment instance.
Testing done: Confirmed the error message by testing with following options:
"--stress_datastream_recvr_delay_ms=90000 datastream_sender_timeout_ms=1000"
Change-Id: Ie3e83773fe6feda057296e7d5544690aa9271fa0
Reviewed-on: http://gerrit.cloudera.org:8080/8751
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
When Lz4Compressor::MaxOutputLen returns 0, it
means that the input is too large to compress.
When invoked Lz4Compressor::ProcessBlock with
an input too large, it silently produced a bogus
result. This bogus result even decompresses
successfully, but not to the data that was
originally compressed.
After this commit, Lz4Compressor::ProcessBlock
will return error if it cannot compress the
input.
I also added a comment on Codec::MaxOutputLen()
that return value 0 means that the input is too
large.
I added some checks after the invocations of
MaxOutputLen() where the compressor can be
a Lz4Compressor.
I added an automated test case to
be/src/util/decompress-test.cc.
Change-Id: Ifb0bc4ed98c5d7b628b791aa90ead36347b9fbb8
Reviewed-on: http://gerrit.cloudera.org:8080/8748
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
KuduRPC has support for Kerberos. However, since Impala's client transport
still uses the Thrift transport stack, we need to make sure that a single
security configuration applies to both internal communication (KuduRPC)
and external communication (Thrift's TSaslTransport).
This patch changes InitAuth() to start Sasl regardless of security
configuration, since KRPC uses plain SASL for negotiation on insecure
clusters.
It also moves some utility code out of authentication.cc into
auth-util.cc for resuse by the RpcMgr while enabling kerberos.
The MiniKDC related code is moved out of thrift-server-test.cc into a
new file called mini-kdc-wrapper.h/cc. This file exposes a new class
MiniKdcWrapper which can be easily used by the tests to configure the
kerberos environment, create the keytab, start the KDC and also
initialize the Impala security library.
Tests are added to rpc-mgr-test for kerberos tests over KRPC.
thrift-server-test also has a mechanical change to use MiniKdcWrapper.
Also tested on a live cluster configured to use kerberos.
Change-Id: I8cec5cca5fdb4b1d46bab19e86cb1a8a3ad718fd
Reviewed-on: http://gerrit.cloudera.org:8080/8270
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Impala Public Jenkins
Before this patch, decimal operations would either silently overflow (in
the case of sum() and avg()), or produce a warning.
In this patch, the behaviour is changed so that an error is produced in
the case of overflow when DECIMAL_v2 is enabled. Decimal v1 behaviour is
unchanged.
We introduce overflow checks when computing sum() and avg(). This
results in a ~30% performance regression when we are in decimal v2 mode
compared to decimal v1.
Benchmarks:
Query:
select sum(dec_38_19) from decimal_tbl
Decimal v1: 11.57s
Decimal v2: 16.58s
Query:
select avg(dec_38_19) from decimal_tbl
Decimal v1: 12.08s
Decimal v2: 17.08s
The performance regression is not as bad if we are computing the sum or
average of decimal column with a lower precision:
Query:
select sum(dec_9_5) from decimal_tbl
Decimal v1: 11.06s
Decimal v2: 13.08s
Query:
select avg(dec_9_5) from decimal_tbl
Decimal v1: 11.56s
Decimal v2: 13.57s
Testing:
- Added several end to end tests.
- Updated Expr tests to check for error in case of overflow.
Change-Id: Id98a92c9a9469ec8cf14e518c741a2dab7053019
Reviewed-on: http://gerrit.cloudera.org:8080/8404
Reviewed-by: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Tested-by: Impala Public Jenkins
