Abhishek Chennaka has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/21768


Change subject: Squash cherry-picked commits #5 This is a combination of 20 
commits.
......................................................................

Squash cherry-picked commits #5
This is a combination of 20 commits.

This is the 1st commit message:

[tablet] modernize MultiColumnWriter's code

This patch contain a few micro-optimizations, but it doesn't contain
any functional changes:
  * std::unique_ptr replaced raw pointers in the cfile_writers_ array,
    so no need to use the STLDeleteElements macro
  * replaced CHECK() with DCHECK() where appropriate
  * added more DCHECK() macros to enforce logical consistency
  * removed virtual base from the class and made it final
  * added tablet identifier into status objects and error messages
    to make it possible to attribute errors to particular tablets

Reviewed-on: http://gerrit.cloudera.org:8080/20702
Tested-by: Alexey Serbin <[email protected]>
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 63dcd56985516e554e8f00474b070b318a598008)

This is the commit message #2:

[cfile] clean up on IndexBlock{Builder,Iterator,Reader}

This patch contains the following updates:

  * the code in IndexBlockReader::Parse() now catches corruption
    of index block's trailer in a more robust manner
  * fixed index overruns and possible memory corruption in
    IndexBlockReader::GetKeyPointer() when working with not-so-expected
    input for an empty BTree file and when the data read from the index
    file is corrupted
  * removed a few unused fields
  * a few class member functions became static
  * added PREDICT_{TRUE,FALSE} macros for better branch prediction
    where appropriate
  * fixed const-correctness
  * changed CHECK() to DCHECK() for the code paths where the
    assertions might trigger due to the variation of control paths,
    but independent of the input data
  * added more DCHECK() assertions where appropriate
  * updated the code to conform to the project's current style guide
  * addressed a few Clang-Tidy warnings
  * other minor updates

I also added a test to cover various error paths in
IndexBlockReader::Parse() when parsing corrupted index blocks.

Reviewed-on: http://gerrit.cloudera.org:8080/20690
Reviewed-by: Ashwani Raina <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
Tested-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 3d5497cf299ee18f937efb9176190c35a33a9215)

This is the commit message #3:

[tablet] NewReplicaOpDriver() --> NewFollowerOpDriver()

This patch renames the NewReplicaOpDriver() method in the TabletReplica
class into NewFollowerOpDriver(), so it's easier to comprehend given
there is the pairing NewLeaderOpDriver() method.  That's a better name
if taking into account the terminology used throughout the project,
where a tablet has a leader and follower replicas in the sense of the
Raft protocol.

This patch doesn't contain any functional modifications.

Reviewed-on: http://gerrit.cloudera.org:8080/20713
Reviewed-by: Mahesh Reddy <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 2efdd37ba8491ac0f55a5f4eaaf2960c62c102ba)

This is the commit message #4:

KUDU-3407 Avoid unchecked scheduling of flush operations.

In some clusters, the memory usages of tservers might be 60% ~ 80%
for a long time. During this time the maintenance manager will not
run any operation other than wal gc and MRS/DRS flushes, which will
make the performance of tservers worse and worse and eventually break
due to OOM.

This patch add an argument to give a chance to do other operations
while server is under memory pressure.

This mechanism works when the memory usage is between
memory_pressure_percentage and memory_limit_soft_percentage.
Higher the memory usage is, higher the probability to flush
MRS/DMS.

e.g.
memory_pressure_percentage = 60%
memory_limit_soft_percentage = 80%
The probability of not flushing MRS/DMS is the value of
run_non_memory_ops_prob. As the memory increases, it gradually
decreases to 0, when thememory usage is 80%.

Reviewed-on: http://gerrit.cloudera.org:8080/20166
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <[email protected]>
(cherry picked from commit 66a79dddf2faf31dbb49cd8e08c0e322c4bb57ce)

This is the commit message #5:

KUDU-3497 optimize leader rebalancer algorithm

Leader rebalancing might not work well for now, especially for
the tables with smaller number of hash partitions.
For instance, for a table, consisting of 9 tablets, RF = 3, in a3-tservers 
cluster.

Its leaders distribution is as follow:

Tablet server A : 4
Tablet server B : 4
Tablet server C : 1

According to the algorithm for now, there will not be any rebalance
operation scheduled.

Therefore, try to find a better algorithm to make it always find
the best leader distribution.

The formula is:
expected leader num = (tablets sum) % (tablets server num) = 0 ?
(tablets sum) / (tablets server num) :
ceil((tablets sum) / (tablets server num))
A tserver whose leader num is more than the expected value needs
to transfer the leaderships.

So the leader skew will never be more than 1.

Reviewed-on: http://gerrit.cloudera.org:8080/20310
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <[email protected]>
(cherry picked from commit d8467c571a5e7eecaf689c9c9647851ce9bf0fd1)

This is the commit message #6:

[thirdparty] update lz4 up to 1.9.4 version

LZ4 v1.9.4 maintenance release includes improvements and fixes.  A few
important highlights are decompression performance improvements:

  * Decompression speed on high-end ARM64 platform is improved by ~+20%
  * For the specific scenario of data compressed with -BD4 setting
    (small blocks, <= 64 KB, linked) decompressed block-by-block
    into a flush buffer, decompression speed is improved ~+70%

See LZ4 1.9.4 release notes [1] for more details.

[1] https://github.com/lz4/lz4/releases/tag/v1.9.4

Reviewed-on: http://gerrit.cloudera.org:8080/20722
Reviewed-by: Mahesh Reddy <[email protected]>
Tested-by: Kudu Jenkins
Reviewed-by: Yifan Zhang <[email protected]>
Reviewed-by: Yingchun Lai <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit cab7e5a19d33091e45d1073b7793807d70570e8d)

This is the commit message #7:

[cfile] remove redundant StringPrintf in status messages

Reviewed-on: http://gerrit.cloudera.org:8080/20740
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Yingchun Lai <[email protected]>
(cherry picked from commit 18ff6d979d74c59b4412c210ec22f4c5772e311d)

This is the commit message #8:

[messenger] Initialize SASL proto name to principal

Adding master in case of custom kerberos principal name didn't work
because the non-default principal name was not propagated to the
MessengerBuilder every time and it used "kudu" instead of the one
provided by the kerberos service. If GetLoggedInUsernameFromKeytab is
available it provided to the MessengerBuilder KuduClientBuilder objects.
If it is not set the sasl_proto_name remains at the default "kudu"
value.
However this change is not enough because ServerBase::ShutdownImpl
called DestroyKerberosForServer which destroyed the global state which
GetLoggedInUsernameFromKeytab uses. The DestroyKerberosForServer call
was moved to ServerBase::~ServerBase().

The new test is using non default principals, adds two new masters, runs
a smoke test. After that one master is shut down to simulate recoverable
master error, and the smoke test is executed again.

Reviewed-on: http://gerrit.cloudera.org:8080/20708
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <[email protected]>
(cherry picked from commit 6e831433239f213f6592b50f15da2317bfc0f84a)

This is the commit message #9:

[ExternalMiniCluster] improve leader_master()

ExternalMiniCluster::leader_master() function was improved to use the
RPC. However during the dns_alias_itest cases, the host that was stored
in the masters_[]->bound_rpc_hostport was a domain name, and the RPC
returns IP address and they are compared as strings. This made
leader_master() check in ExternalMiniCluster::AddTabletServer() fail.
The solution is that bound_rpc_hostport values are converted to Sockaddr
and compared to the leader_master_addr that is retrieved from the
ConnectToClusterRpc call.

ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys was changed so
it doesn't start any tservers, since leader_master() check can't find
leader during tablet server initialization.

In master_cert_authority-itest, num of tablet servers was set to zero as
well because tablet servers are not needed for the test and waiting for
leader made the test flaky.

Reviewed-on: http://gerrit.cloudera.org:8080/20714
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <[email protected]>
(cherry picked from commit 158cf0fa00b86cdaf75f60e520c99920bbe6c8c3)

This is the commit message #10:

[java] sync slow DNS resolution threshold with the C++ code

Prior to this patch, the threshold for slow DNS resolution warning
was set to just 3 ms in the code of the Kudu Java client, which didn't
make a lot of sense if talking about non-cached DNS queries.

This patch synchronizes the warning threshold with the corresponding
threshold in the Kudu's C++ code (200 ms), and also doubles the debug
threshold, bumping up the latter from 0.5 ms to 1 ms.

Reviewed-on: http://gerrit.cloudera.org:8080/20743
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 9ede74b8a575be4185a73530181e8714a1f1b322)

This is the commit message #11:

[tools] disambiguate parse_metrics' output

This patch improves the output of the 'kudu diagnostic parse_metrics'
CLI tool.  Prior to this patch, it was impossible to tell between
absent metric values at a particular timestamp and zeros.  With this
patch, an absent value and undefined rate of change of a metric for
a particular timestamp are indicated with the '?' ASCII character.

Reviewed-on: http://gerrit.cloudera.org:8080/20741
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 7f1bfb59210d8298aacbaa6ce2937adb359d802e)

This is the commit message #12:

more strict check for [un]setenv() return code

Even if setenv() and unsetenv() function expected to almost never fail
with the current code as per documentation [1], it still makes sense
to check for their return code. This patch updates the call sites
of these functions accordingly.

I also addressed a few warnings that ClangTidy produced
on the updated code.

[1] https://man7.org/linux/man-pages/man3/setenv.3.html

Reviewed-on: http://gerrit.cloudera.org:8080/20744
Reviewed-by: Abhishek Chennaka <[email protected]>
Reviewed-by: Mahesh Reddy <[email protected]>
Tested-by: Kudu Jenkins
Reviewed-by: Yifan Zhang <[email protected]>
(cherry picked from commit bc4c70c78435bd65a385eeb22f3894457d0b2e3a)

This is the commit message #13:

[security] a small style-related code cleanup

Since I'm working on adding new functionality into MiniKdc, I separated
this code-style only part of recent updates into a separate patch.

This patch doesn't contain any functional modifications.

Reviewed-on: http://gerrit.cloudera.org:8080/20745
Reviewed-by: Mahesh Reddy <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
(cherry picked from commit 50f8729860e6e8e3081ea3de5fcc5c016d4b2215)

This is the commit message #14:

[util] a few test scenarios for KLOG_EVERY_N_SECS() macro

This patch adds a couple of extra test scenarios on the behavior
of the KLOG_EVERY_N_SECS() macro.

Reviewed-on: http://gerrit.cloudera.org:8080/20757
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 864c26007625397d5edd4e5b3b9205545a3a5be1)

This is the commit message #15:

[util] make Socket::SetTcpKeepAlive() available on macOS

Reviewed-on: http://gerrit.cloudera.org:8080/20765
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Mahesh Reddy <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit a69d9b197a4ef1290e6931a75bd28257d949b315)

This is the commit message #16:

[rpc] micro-optimizations on RpczStore::LogTrace

This patch micro-optimizes the code in RpczStore::LogTrace():
  * faster computation of the logging threshold
  * std::string instance is no longer allocated when outputting
    the trace of an RPC call
  * the LogTrace() method is now static
As an extra style-related change, 'final' specifier has been added to
the RpczStore class' declaration since it was not meant for adding
derived classes on top (its destructor wasn't virtual).  Also, switched
samples' timestamp from Atomics to std::atomics and did other unsorted
changes to avoid memory allocation while holding a spinlock and shorten
duration of critical sections overall.

This patch also contains a few nano-optimizations for AsyncLogger class:
  * 'final' specifier was added to allow for more run-time optimizations
  * unsorted minor changes

Reviewed-on: http://gerrit.cloudera.org:8080/20748
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 66815148adb79cdd25cfc0399dc1e1fa9622bff5)

This is the commit message #17:

[rpc] modernize AcceptorPool's code

Since I'm modifying AcceptorPool implementation to add a new metric,
I went ahead and modernized the code a bit to logically separate
these mostly style-related updates from follow-up modifications.
This patch:
  * replaces Atomic32 field with std::atomic<bool>
  * updates the code to make it conform to the project style guide
  * other minor improvements

There are no functional modifications in this changelist.

Reviewed-on: http://gerrit.cloudera.org:8080/20789
Reviewed-by: Abhishek Chennaka <[email protected]>
Tested-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit a7e0a36dda2fd2c0545fa8f00f4bc1711059224d)

This is the commit message #18:

[rpc] add metric for AcceptorPool's dispatch timing

This patch adds 'acceptor_dispatch_times' histogram metric to track
dispatching times of newly accepted connections by AcceptorPool along
with a very basic unit test for the newly added metric.

Reviewed-on: http://gerrit.cloudera.org:8080/20790
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 2a07d95986f125b9186bff183f03d544a441d64d)

This is the commit message #19:

KUDU-3524 Fix crash when sending periodic keep-alive requests

Currently, Kudu client applications on macOS crash upon calling
StartKeepAlivePeriodically(), see KUDU-3524 for details. That's
because a PeriodicTimer was used to send keep-alive requests in
a synchronous manner, while attempting to wait for the response
on a reactor thread. However, reactor threads do not allow for
waiting.

This patch uses 'ScannerKeepAliveAysnc()', an asynchronous
interface to send keep-alive requests to avoid this problem.

Reviewed-on: http://gerrit.cloudera.org:8080/20739
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
(cherry picked from commit 6bcda5eff94ea7c7f96c38d67ade3f83111e6743)

This is the commit message #20:

[tserver] unify logging on rejected write requests

This patch contain a minor clean up on the logging of rejected write
requests and other minor improvements, making the logs more useful
for post-mortem analysis and troubleshooting in some scenarios:
  * unified the prefix for log messages on rejected write requests,
    so it's now easier to search in the logs
  * log about all rejected write requests using time-based throttling
    wrapper KLOG_EVERY_N_SECS(.., 1)
  * log about all rejected write requests using INFO severity unless
    going over the --memory_limit_warn_threshold_percentage threshold
    for memory pressure rejections; use WARNING severity for the latter
  * changed TabletServerErrorPB from UNKNOWN_ERROR to THROTTLED when
    rejecting a write request due to a throttling condition
  * use static instances of Status objects to avoid allocating and
    freeing memory for status messages upon rejections

Reviewed-on: http://gerrit.cloudera.org:8080/20808
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <[email protected]>
(cherry picked from commit 8bc0be58ef0a02f33d2ecd71cfe721fd16db1730)

Change-Id: I909322ab146efb527647be2e25c135ca65063986
---
M java/kudu-client/src/main/java/org/apache/kudu/util/NetUtil.java
M src/kudu/benchmarks/tpch/tpch_real_world.cc
M src/kudu/cfile/binary_plain_block.cc
M src/kudu/cfile/binary_plain_block.h
M src/kudu/cfile/index-test.cc
M src/kudu/cfile/index_block.cc
M src/kudu/cfile/index_block.h
M src/kudu/cfile/index_btree.cc
M src/kudu/cfile/index_btree.h
M src/kudu/client/client-test.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/client/scanner-internal.h
M src/kudu/integration-tests/master_cert_authority-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/master/auto_leader_rebalancer-test.cc
M src/kudu/master/auto_leader_rebalancer.cc
M src/kudu/master/auto_rebalancer.cc
M src/kudu/master/master_runner.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/mini-cluster/internal_mini_cluster.cc
M src/kudu/mini-cluster/internal_mini_cluster.h
M src/kudu/mini-cluster/mini_cluster.h
M src/kudu/rpc/acceptor_pool.cc
M src/kudu/rpc/acceptor_pool.h
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/rpcz_store.cc
M src/kudu/rpc/rpcz_store.h
M src/kudu/security/init.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/security/test/mini_kdc.h
M src/kudu/server/server_base.cc
M src/kudu/server/webserver-test.cc
M src/kudu/tablet/multi_column_writer.cc
M src/kudu/tablet/multi_column_writer.h
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tools/diagnostics_log_parser.cc
M src/kudu/tools/diagnostics_log_parser.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/util/async_logger.cc
M src/kudu/util/async_logger.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/maintenance_manager-test.cc
M src/kudu/util/maintenance_manager.cc
M src/kudu/util/maintenance_manager.h
M src/kudu/util/net/socket.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
M thirdparty/vars.sh
54 files changed, 1,717 insertions(+), 688 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.17.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I909322ab146efb527647be2e25c135ca65063986
Gerrit-Change-Number: 21768
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>

Reply via email to