Skip to content

RFCs: propose a syntax for pausing/resuming/canceling operations#16273

Merged
benesch merged 1 commit intocockroachdb:masterfrom
benesch:pause-resume-cancel
Jun 8, 2017
Merged

RFCs: propose a syntax for pausing/resuming/canceling operations#16273
benesch merged 1 commit intocockroachdb:masterfrom
benesch:pause-resume-cancel

Conversation

@benesch
Copy link
Copy Markdown
Contributor

@benesch benesch commented Jun 1, 2017

No description provided.

@rjnn
Copy link
Copy Markdown
Contributor

rjnn commented Jun 1, 2017

This change is Reviewable

@petermattis
Copy link
Copy Markdown
Collaborator

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


docs/RFCS/pause_resume_cancel.md, line 92 at r1 (raw file):

```sql
ALTER SYSTEM KILL SESSION <session-id>;

ALTER JOB [PAUSE|RESUME|CANCEL] seems more natural to me, though I'm not opposed to adding new top-level syntax.


Comments from Reviewable

@bdarnell
Copy link
Copy Markdown
Contributor

bdarnell commented Jun 1, 2017

:lgtm:

I'm fine with either the proposed syntax or Peter's ALTER (QUERY|JOB) (CANCEL|PAUSE|RESUME)


Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


docs/RFCS/pause_resume_cancel.md, line 71 at r1 (raw file):

rolls back all backfill progress.

## Postgres syntax

Postgres additionally has support for cancellation built in to the wire protocol: send a special packet on a new TCP connection (bypassing all the regular authentication. in its place, you pass a token that the server sends at the start of each connection).

I have no idea how common each mode of operation is in practice. My guess is that only very postgres-specific tools like psql use the protocol-based mode, and anything more generic would use the function.


docs/RFCS/pause_resume_cancel.md, line 84 at r1 (raw file):

seem like they *should* have new syntax.

Alas, we may eventually end up supporting this syntax for compatibility's sake.

What type did we end up using as our query ids? Integers or bytes/strings? We may only be able to support pg_cancel_backend if we use integer query ids.


Comments from Reviewable

@itsbilal
Copy link
Copy Markdown
Contributor

itsbilal commented Jun 1, 2017

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


docs/RFCS/pause_resume_cancel.md, line 84 at r1 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

What type did we end up using as our query ids? Integers or bytes/strings? We may only be able to support pg_cancel_backend if we use integer query ids.

We haven't decided on that yet, but good point - we may want to stick to integer counters as query IDs for this reason alone. We can also discuss this around the time my query cancellation RFC is out for review; it doesn't have to be finalized for this RFC.


Comments from Reviewable

@benesch
Copy link
Copy Markdown
Contributor Author

benesch commented Jun 1, 2017 via email

@bdarnell
Copy link
Copy Markdown
Contributor

bdarnell commented Jun 1, 2017

Postgres "backend IDs" used for cancellation are 32 bits.

@a-robinson
Copy link
Copy Markdown
Contributor

:lgtm: Thanks for including how different systems approach the problem!


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


Comments from Reviewable

@jordanlewis
Copy link
Copy Markdown
Member

:lgtm: IMO, CANCEL/PAUSE/RESUME is more natural than ALTER QUERY/JOB.


Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


Comments from Reviewable

@benesch
Copy link
Copy Markdown
Contributor Author

benesch commented Jun 5, 2017

Since it seems no one is opposed to CANCEL/PAUSE/RESUME and we have a few votes in favor of it: entering final comment period!

@vivekmenezes
Copy link
Copy Markdown
Contributor

:lgtm:


Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


Comments from Reviewable

@benesch benesch force-pushed the pause-resume-cancel branch from c005d59 to 3f2b8b5 Compare June 8, 2017 00:08
@benesch benesch force-pushed the pause-resume-cancel branch from 3f2b8b5 to 4840529 Compare June 8, 2017 02:09
@benesch
Copy link
Copy Markdown
Contributor Author

benesch commented Jun 8, 2017

Hearing no objections, I'm merging!

@benesch benesch merged commit 9ca34ef into cockroachdb:master Jun 8, 2017
@benesch benesch deleted the pause-resume-cancel branch June 8, 2017 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants