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

refactor(executor): experimental support of cancellable query execution. #467

Merged

Conversation

arkbriar
Copy link
Contributor

@arkbriar arkbriar commented Feb 14, 2022

I'm going to close #422 because it's far more complicated to interrupt a query and make things right when a query is cancelled, such as aborting/rolling back transactions and cleaning up spawned tasks.

This PR is not only to #423. Here're the main contributions of this PR:

  1. Use cancellation token to signal and detect the cancellation.
  2. Provides a non-invasive interface cancellable to provide the cancellation of executors and integrates it inside the executor tree.
  • IMO, only the leaf nodes with much IO works should be sensitive to the cancellation.
  • Resources such as transactions are rollbacked in a spawned task. We think spawned tasks as completable tasks as long as the async runtime is still running. This avoids the complex vacuum thread and cleans the room ASAP.
  1. Provide a struct and a set of interfaces to organize the spawned tasks during execution, which avoids leaks while shutting down the async runtime (exit process immediately after interrupting the query).

Here I only include the cancellation of TableScanExecutor in this PR. However, the others (e.g. insert/delete/copy) can be supported in the same fashion.

Now you can interrupt a select query in the CLI at any time as you type the CTRL-C. The transactions shall be dealt properly.

To show the debug messages below, please run with RUST_LOG=risinglight=debug cargo run --release.

RUST_LOG=risinglight=debug cargo run --release
> select
    l_orderkey,
    sum(l_extendedprice * (1 - l_discount)) as revenue,
    o_orderdate,
    o_shippriority
from
    customer,
    orders,
    lineitem
where
    c_mktsegment = 'BUILDING'
    and c_custkey = o_custkey
    and l_orderkey = o_orderkey
    and o_orderdate < date '1995-03-15'
    and l_shipdate > date '1995-03-15'
group by
    l_orderkey,
    o_orderdate,
    o_shippriority
order by
    revenue desc,
    o_orderdate
limit 10;

...
^CInterrupted
2022-02-14T15:15:20.381454Z  WARN risinglight::executor::table_scan: Abort!
2022-02-14T15:15:20.381506Z DEBUG risinglight::db: error: abort

Cargo.toml Outdated Show resolved Hide resolved
@@ -114,20 +120,89 @@ pub type BoxedExecutor = BoxStream<'static, Result<DataChunk, ExecutorError>>;
/// The builder of executor.
#[derive(Clone)]
pub struct ExecutorBuilder {
context: Arc<Context>,
storage: StorageImpl,
}

impl ExecutorBuilder {
/// Create a new executor builder.
pub fn new(storage: StorageImpl) -> ExecutorBuilder {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we even need the method now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, just for backward compatibility. Remove it should be nicer.

Copy link
Collaborator

@TennyZhuang TennyZhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Generally LGTM, but it seems that we may need some more comments.

@TennyZhuang
Copy link
Collaborator

And please resolve conflicts.

arkbriar and others added 6 commits February 16, 2022 18:44
Signed-off-by: arkbriar <arkbriar@gmail.com>
Signed-off-by: TennyZhuang <zty0826@gmail.com>
Signed-off-by: TennyZhuang <zty0826@gmail.com>
Signed-off-by: arkbriar <arkbriar@gmail.com>
Signed-off-by: arkbriar <arkbriar@gmail.com>
Signed-off-by: arkbriar <arkbriar@gmail.com>
@arkbriar arkbriar force-pushed the feat/experimental-cancel-query branch from edcc796 to bf1f57d Compare February 16, 2022 12:08
@arkbriar
Copy link
Contributor Author

Great work! Generally LGTM, but it seems that we may need some more comments.

Thanks @TennyZhuang . I am greatly encouraged.

I added more commends/docs on SealableAtomicCounter and WaitGroup and created a new package called utils and moved the sync package to be under the utils.

Also, 4 more executors (insert/delete, copy from/to file) are refactored and they are all cancellable now. The leaf executors are all cancellable and I think we can interrupt any query from now on.

And please resolve conflicts.

All conflicts should be resolved now.

Please review this again when you have time.

@TennyZhuang
Copy link
Collaborator

Thanks! I think it’s good enough to merge!

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.

2 participants