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

jobs,importccl: adopt protected timestamps for IMPORT #44739

Commits on Feb 29, 2020

  1. jobs: create a StartableJob.

    StartableJob isn't great but it's better than what we've got now.
    A StartableJob is a job which is created transactionally but also hooked in
    to a `resultCh` and can be started on the local gateway after the transaction
    commits.
    
    Currently we've got two main ways that jobs are kicked off "synchronously":
    
     1) Registry.Run()
    
    This doesn't currently do anything to kick off the jobs - it just waits for
    all of the relevant jobs to be started. This is used in conjunction with
    `planner.QueueJob` and `CreateJobWithTxn`. The idea here is that when a
    transaction creates a job we want, upon commit, for it to wait for the
    jobs it created to complete.
    
     2) Registry.CreateAndStartJob()
    
    This method has special powers that prevents the gateway node from adopting
    the job and also writes the job with a lease for the local node. It also allows
    the caller to pass a resultsCh which will recieve datums from the job.
    
    That resultCh is critical for IMPORT and BACKUP (?). Currently in both of those
    cases we create and run the job on a transaction outside of the caller's txn.
    
    This is particularly frustrating because the jobs package assigns IDs at
    creation time, making it difficult to transactionally associate a job with
    another resource while also retaining the ability to ensure that it starts
    locally and is connected to a resultsCh. This commit addresses that.
    
    The fact that `IMPORT` and `BACKUP` commit the job before committing the
    planner's txn feels dirty. I'm not planning on mucking with that stuff here in
    this PR but ultimately the work here seems to lay a possible path forward. In
    particular, instead of calling `Run()` with a slice of IDs we could call it
    with a slice of `StartableJob`s which may be hooked up to the channel. In that
    way we could unify CreateAndStartJob() and Run() with something better.
    
    Release note: None.
    ajwerner committed Feb 29, 2020
    Configuration menu
    Copy the full SHA
    8e13d4f View commit details
    Browse the repository at this point in the history
  2. importccl: adopt protected timestamps for IMPORT

    Perhaps this only needed to be applied to IMPORT INTO but it doesn't hurt
    import. This change ensures that IMPORT statements protect the spans into
    which they'll be writing from being GC'd.
    
    The change adopts the StartableJob change from the previous commit,
    protects a timestamp, and then makes sure to clean up that timestamp
    on success or failure.
    
    Before this change the test added here would fail with the following error:
    
    ```
    require.go:1104:
                    Error Trace:    import_into_test.go:152
                    Error:          Expect "pq: job 526896149330362369: cannot be reverted, manual cleanup may be required: rolling back partially completed IMPORT: cannot revert before replica GC threshold 1580866345.938283735,0" to match "error response from server: 500 Internal Server Error"
                    Test:           TestProtectedTimestampsDuringImportInto
    ```
    
    Release note (bug fix): IMPORT INTO jobs which are canceled or fail
    can no longer get stuck in an unrecoverable state if data from the
    previous state of the table had expired relative to the GC TTL.
    ajwerner committed Feb 29, 2020
    Configuration menu
    Copy the full SHA
    46ab253 View commit details
    Browse the repository at this point in the history
  3. prototutil: provide mechanism for packages to register unclonable types

    The logic to detect whether a type is clonable cannot traverse into all of the
    implementations of itnerfaces. This is problematic when one of the structs
    underneath a oneof is unclonable. While a more general solution might be nice,
    this workaround allows the addition of a UUID field to a job details struct.
    
    Release note: None
    ajwerner committed Feb 29, 2020
    Configuration menu
    Copy the full SHA
    bfa371b View commit details
    Browse the repository at this point in the history