Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improved Txn restart behaviour #221

Merged
merged 5 commits into from Dec 20, 2014
Merged

improved Txn restart behaviour #221

merged 5 commits into from Dec 20, 2014

Commits on Dec 18, 2014

  1. collect NodeIDs for ReadWithinUncertainty restarts

    the NodeIDs are stored within the Transaction associated to the request.
    After having stored a node, future read requests within the same transaction
    can set MaxTimestamp = Timestamp in order to prevent uncertainty related
    restarts.
    
    Implementing this should considerably impact performance in the relevant case
    of transaction reading a busy key from a node with a fast clock, which
    otherwise would lead to repeated restarts until MaxTimestamp passes.
    tbg committed Dec 18, 2014
    Configuration menu
    Copy the full SHA
    e8f06e3 View commit details
    Browse the repository at this point in the history
  2. no Txn restarts for known "certain" hosts

    Building on the last commit, this implements intelligent handling of
    MaxTimestamp, taking into account a prior uncertainty related restarts.
    In such a case, the host is known to the Txn via its NodeID, and the
    request may be carried out with Timestamp = MaxTimestamp, preventing
    a further uncertainty related restart
    tbg committed Dec 18, 2014
    Configuration menu
    Copy the full SHA
    4fc618b View commit details
    Browse the repository at this point in the history
  3. add test for uncertainty restart behaviour

    TestUncertaintyRestarts verifies that transactional reads within the
    uncertainty interval cause exactly one restart. The test runs a transaction
    which attempts to read a single key, but just before that read, a future
    version of that key is written directly through the MVCC layer.
    
    Indirectly this tests that the transaction remembers the NodeID of the node
    being read from correctly, at least in this simple case. Not remembering the
    node would lead to thousands of transaction restarts.
    
    This can easily be tested by disabling that code in the local sender and
    running the test.
    tbg committed Dec 18, 2014
    Configuration menu
    Copy the full SHA
    8ab3048 View commit details
    Browse the repository at this point in the history
  4. Simple transaction benchmark

    The benchmark runs N transactions back to back, each writing once to the same
    key.
    
    I've been seeing a bunch of WriteIntentError related restarts, supposedly
    because cleaning up the intents happens asynchronously. This looks like it's
    something to expect in such a benchmark to me.
    
    Once Raft is in place, likely the bottleneck will be that.
    tbg committed Dec 18, 2014
    Configuration menu
    Copy the full SHA
    928b212 View commit details
    Browse the repository at this point in the history

Commits on Dec 20, 2014

  1. address @spencerkimball's feedback

    notably,
    - the range will forward an uncertain read error using the local node's clock,
      as per spencerkimball's comment
    - added a test for that
    - updated comments in all affected files to point to the proto.Transaction
      comment for details on this somewhat intricate interplay.
    tbg committed Dec 20, 2014
    Configuration menu
    Copy the full SHA
    a6afc9b View commit details
    Browse the repository at this point in the history