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

spanconfig/sqlwatcher: hide factory pattern, speed up tests, surface no-op checkpoints periodically #73171

Merged
merged 4 commits into from
Nov 30, 2021

Commits on Nov 25, 2021

  1. spanconfig/sqlwatcher: speed up tests

    Using a more aggressive closed timestamp target duration brings the
    runtime for TestSQLWatcherReactsToUpdate down from 45s+ to single
    digits. To speed it up, we also just re-use the same sqlwatcher instead
    of creating a new one for every test case.
    
    Other tests in the package benefit from a similar treatment. Using the
    default target duration in fact masked a buggy test; in a future commit
    we end up rewrite that test so it's skipped for now.
    
    Release note: None
    irfansharif committed Nov 25, 2021
    Configuration menu
    Copy the full SHA
    ee68771 View commit details
    Browse the repository at this point in the history
  2. spanconfig/sqlwatcher: hide the factory pattern

    The only mutable state across concurrent WatchForSQLUpdate calls was the
    internal buffer, which does not need to hang off the surrounding struct.
    This obviates the need for the factory pattern we were using -- callers
    can set up multiple SQLWatchers concurrently as is (see
    TestSQLWatcherMultiple).
    
    This commit simply hides the factory under the package boundary; in a
    future commit we'll shed it altogether. This has the benefit of reducing
    the number of symbols in pkg/spanconfig and making it symmetric with the
    other spanconfig dependencies typically found together (KVAccessor,
    SQLTranslator). It's also every-so-slightly less verbose to use in the
    upcoming spanconfig.Reconciler (cockroachdb#71994).
    
    Release note: None
    irfansharif committed Nov 25, 2021
    Configuration menu
    Copy the full SHA
    46c7597 View commit details
    Browse the repository at this point in the history
  3. spanconfig/sqlwatcher: remove internal factory pattern

    A previous commit obviated the need for a factory altogether, but went
    only as far hiding it within the package boundary. This commit gets rid
    of it entirely.
    
    Release note: None
    irfansharif committed Nov 25, 2021
    Configuration menu
    Copy the full SHA
    6fdc475 View commit details
    Browse the repository at this point in the history

Commits on Nov 30, 2021

  1. spanconfig/sqlwatcher: checkpoint, even without updates

    If `system.{descriptor,zones}` is static, we would previously discard
    all checkpoint events. This is not what we want -- we'd still like to
    know how caught up we are, with ongoing updates or otherwise.
    
    MVCC GC, after all, can still happen; if our last checkpoint was when
    the last update occurred, we may end up doing a lot of unnecessary work
    when finding out our last checkpoint was GC-ed from underneath us. Lack
    of periodic checkpoints also makes tests difficult to write -- you'd
    have to induce a benign update to flush out all earlier ones. This
    latent flakiness was uncovered after speeding up some existing tests in
    an earlier commit.
    
    NB: For incremental (possibly noop) checkpoints, we need to ensure that
    the sqlwatcher's buffer is initialized with a low watermark. When
    flushing events, we take the most conservative timestamp. If not
    initialized to a high water, this might be 0 -- violating the
    sqlwatcher's monotonically increasing checkpoint ts invariant.
    
    Release note: None
    irfansharif committed Nov 30, 2021
    Configuration menu
    Copy the full SHA
    d00baeb View commit details
    Browse the repository at this point in the history