Hello Tidy Bot, Zoltan Chovan, Yuqi Du, Ashwani Raina, Yingchun Lai, Attila 
Bukor, Kudu Jenkins,

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

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

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

Change subject: [clock] add sanity check to detect wall clock jumps
......................................................................

[clock] add sanity check to detect wall clock jumps

There was a case when a timestamp read from system/local clock using
the ntp_adjtime() call jumped 40+ years ahead when running kudu-tserver
on an Azure VM, while ntp_adjtime() didn't report an error on clock
being unsynchronized. That came along with a huge number of kernel
messages, and other software (such as the SASL library used by SSSD)
detected the strange jump in the local clock as well.  My multiple
attempts to reproduce the issue on a real hardware node, Dockerized
environment run at a real hardware server in a datacenter, and GCE & EC2
VMs were not successful.

This patch adds a sanity check to detect such strange jumps in wall
clock readings.  The idea is to rely on the readings from the
CLOCK_MONOTONIC clock captured along with the wall clock readings.
A jump should manifest itself in big difference between the wall clock
delta and the corresponding CLOCK_MONOTONIC delta.  If such a condition
is detected, then HybridClock::NowWithErrorUnlocked() dumps diagnostic
information about clock synchronisation status and aborts.

This patch also adds a unit test for the newly added functionality.

As a part of this changelist, the following new flags are introduced:
  * --wall_clock_jump_detection
      The flag to control the newly introduced sanity check for readings
      of the wall clock.  Acceptable values are 'auto', 'enabled,
      and 'disabled'.  Set to 'auto' by default, meaning the sanity
      check for timestamps is enabled if the process detects that it's
      running on an Azure VM.
  * --wall_clock_jump_threshold_s
      The threshold (in seconds) for the difference in corresponding
      deltas of the wall clock's and CLOCK_MONOTONIC_RAW clock's
      readings.  Set to 900 (15 minutes) by default.

The reasoning behind having --wall_clock_jump_detection=auto by
default is to skip an extra check at the majority of nodes out there
since NTP-synchronized system clock isn't supposed to jump that much
at all.  However, since a problem is observed at some inadequate VMs
such as ones in Azure cloud, it's now enabled automatically on such
nodes to detect such issues.  If the clock jump went unnoticed, the
timestamp would be persisted with an operation in the WAL and propagated
to other replicas.  That could lead to crashes during tablet
bootstrapping, and would require manual intervention to remove
the orphaned operations with out-of-whack timestamps from the WAL.

To assess the performance hit of the jump detection, I also added
a parameterized HybridClockJumpProtectionTest.BasicPerf scenario
to compare how fast HybridClock::NowWithError() method runs with
and without the clock jump detection.

I found the difference in runtimes to be non-material, especially for
the RELEASE build.  The details are below.

I used the command below, running it 10 times for DEBUG and RELEASE
builds to run the test scenarios.  I recorded the timings output by
the test scenarios and computed the average time taken to call invoke
HybridClock::NowWithError() 2000000 times.

  KUDU_ALLOW_SLOW_TESTS=1 ./bin/hybrid_clock-test --gtest_filter='*Perf*'

  DEBUG build:

  without clock jump detection:

    2000000 iterations in 1299.592535 ms
    2000000 iterations in 1232.211616 ms
    2000000 iterations in 1247.237539 ms
    2000000 iterations in 1242.360072 ms
    2000000 iterations in 1368.855273 ms
    2000000 iterations in 1334.929966 ms
    2000000 iterations in 1325.736948 ms
    2000000 iterations in 1332.034905 ms
    2000000 iterations in 1291.398501 ms
    2000000 iterations in 1308.596095 ms

    average: 1298.2953450 ms

  with clock jump detection:

    2000000 iterations in 1281.678966 ms
    2000000 iterations in 1230.120822 ms
    2000000 iterations in 1295.044968 ms
    2000000 iterations in 1234.285532 ms
    2000000 iterations in 1365.331334 ms
    2000000 iterations in 1358.448343 ms
    2000000 iterations in 1336.828018 ms
    2000000 iterations in 1352.825380 ms
    2000000 iterations in 1260.334167 ms
    2000000 iterations in 1343.749751 ms

    average: 1305.8647281 ms

  -------------------------------------------------------------------

  RELEASE build:

  without clock jump detection:

    2000000 iterations in 288.288766 ms
    2000000 iterations in 288.252244 ms
    2000000 iterations in 289.027414 ms
    2000000 iterations in 288.729931 ms
    2000000 iterations in 294.876418 ms
    2000000 iterations in 288.053229 ms
    2000000 iterations in 291.324382 ms
    2000000 iterations in 289.249022 ms
    2000000 iterations in 288.665515 ms
    2000000 iterations in 288.629355 ms

    average: 289.5096276 ms

  with clock jump detection:

    2000000 iterations in 288.331674 ms
    2000000 iterations in 288.380279 ms
    2000000 iterations in 289.147557 ms
    2000000 iterations in 288.010613 ms
    2000000 iterations in 294.012716 ms
    2000000 iterations in 288.398767 ms
    2000000 iterations in 288.180751 ms
    2000000 iterations in 288.699496 ms
    2000000 iterations in 288.403666 ms
    2000000 iterations in 288.395125 ms

    average: 288.9960644 ms

Change-Id: I630783653717d975a9b2ad668e8bd47b7796d275
---
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/clock/system_ntp.h
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
6 files changed, 374 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/19473/5
--
To view, visit http://gerrit.cloudera.org:8080/19473
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I630783653717d975a9b2ad668e8bd47b7796d275
Gerrit-Change-Number: 19473
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ara...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org>
Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zcho...@cloudera.com>

Reply via email to