seastar
seastar copied to clipboard
scylladb
abort source: add abort_on_{any,all} combiners
abort_on_any requests abort when abort is requested on any of the provided abort sources. abort_on_all requests abort when abort is requested on all of the provided abort sources.
abort_when_xx sounds like a function, not a class.
I don't know how to feel about this. It looks like a maze of small features, for something that should be easy to use and understand. This is used on rare paths, so it shouldn't require a deep study to figure out.
abort_when_xx sounds like a function, not a class.
I don't know how to feel about this. It looks like a maze of small features, for something that should be easy to use and understand. This is used on rare paths, so it shouldn't require a deep study to figure out.
This functionality could be composed by ad-hoc functions, e.g. for abort_when_either using two "input" abort sources, a condition_variable that's signaled by their subscription callbacks and an "output" abort_source to push the combined request_abort further down the stack.
abort_when_xx sounds like a function, not a class. I don't know how to feel about this. It looks like a maze of small features, for something that should be easy to use and understand. This is used on rare paths, so it shouldn't require a deep study to figure out.
This functionality could be composed by ad-hoc functions, e.g. for abort_when_either using two "input" abort sources, a condition_variable that's signaled by their subscription callbacks and an "output" abort_source to push the combined request_abort further down the stack.
I don't want to solve the problem by making users duplicate glue code. But I also don't want a very wide API that has lots of options, resulting in something no-one remembers how it works.
I'm looking for a simple an intuitive API, but we are in a bind because we allowed abort_source to become part of the data plane (in semaphore) and now we don't want to enlarge its footprint or slow it down.
An intuitive (IMO) API would be to have an abort_source constructor that accepts a parent abort_source and subscribes to it.
Perhaps we should discourage transient abort_source use in semaphore by deprecating it, and move the users to use a condition_variable instead. Long queues are better managed by custom code rather than overloading a semaphore. @denesb please share your experience here.
abort_when_xx sounds like a function, not a class. I don't know how to feel about this. It looks like a maze of small features, for something that should be easy to use and understand. This is used on rare paths, so it shouldn't require a deep study to figure out.
This functionality could be composed by ad-hoc functions, e.g. for abort_when_either using two "input" abort sources, a condition_variable that's signaled by their subscription callbacks and an "output" abort_source to push the combined request_abort further down the stack.
I don't want to solve the problem by making users duplicate glue code. But I also don't want a very wide API that has lots of options, resulting in something no-one remembers how it works.
I'm looking for a simple an intuitive API, but we are in a bind because we allowed abort_source to become part of the data plane (in semaphore) and now we don't want to enlarge its footprint or slow it down.
An intuitive (IMO) API would be to have an abort_source constructor that accepts a parent abort_source and subscribes to it.
Perhaps we should discourage transient abort_source use in semaphore by deprecating it, and move the users to use a condition_variable instead. Long queues are better managed by custom code rather than overloading a semaphore. @denesb please share your experience here.
Why do you think those primitives are needed because of the way semaphore uses aborts_source? @bhalevy need it not related to the semaphore in any way. What if you want to abort an operation for one of two reasons: because Scylla is shutting down or because an administrator (or some internal logic) decided to abort only this particular operation? You have two abort_sources one subscribed to a stop another is for the operation you want to abort and you want to chain them. This is done manually today, but with a proper primitive it will become easier.
In v2 (79a3b28):
- added abort_source member
- changed abort_source& members to abort_source*
- initted to nullptr
- allow default ctor
- added default constructors
- added move constructors and operator
- plus unit tests
- re-subscribe after move as the old subscriptions operate on the rvalue _as member
In v3 (79a3b28):
- renamed abort_when_xx to abort_on_xx (following abort_on_expiry and abort_on_badf)
- extended use of seastar::abort_source to disambiguate the type from the abort_source() member for older compiler versions.
abort_when_xx sounds like a function, not a class. I don't know how to feel about this. It looks like a maze of small features, for something that should be easy to use and understand. This is used on rare paths, so it shouldn't require a deep study to figure out.
This functionality could be composed by ad-hoc functions, e.g. for abort_when_either using two "input" abort sources, a condition_variable that's signaled by their subscription callbacks and an "output" abort_source to push the combined request_abort further down the stack.
I don't want to solve the problem by making users duplicate glue code. But I also don't want a very wide API that has lots of options, resulting in something no-one remembers how it works. I'm looking for a simple an intuitive API, but we are in a bind because we allowed abort_source to become part of the data plane (in semaphore) and now we don't want to enlarge its footprint or slow it down. An intuitive (IMO) API would be to have an abort_source constructor that accepts a parent abort_source and subscribes to it. Perhaps we should discourage transient abort_source use in semaphore by deprecating it, and move the users to use a condition_variable instead. Long queues are better managed by custom code rather than overloading a semaphore. @denesb please share your experience here.
Why do you think those primitives are needed because of the way semaphore uses aborts_source?
We need to keep abort_source slim because semaphore uses it, and semaphore is a low-level primitive.
If we could declare that abort_source was a high-level primitive with just a few objects constructed and destroyed over the lifetime of the application, we could focus on its usability rather than its footprint.
@bhalevy need it not related to the semaphore in any way. What if you want to abort an operation for one of two reasons: because Scylla is shutting down or because an administrator (or some internal logic) decided to abort only this particular operation? You have two abort_sources one subscribed to a stop another is for the operation you want to abort and you want to chain them. This is done manually today, but with a proper primitive it will become easier.
I want it to be done without an additional class, just an overload to abort_source constructor taking a higher scope abort_source. This was @bhalevy initial approach, which I liked, but then he switch to this one due to (real) concerns about increasing semaphore's footprint.
abort_when_xx sounds like a function, not a class. I don't know how to feel about this. It looks like a maze of small features, for something that should be easy to use and understand. This is used on rare paths, so it shouldn't require a deep study to figure out.
This functionality could be composed by ad-hoc functions, e.g. for abort_when_either using two "input" abort sources, a condition_variable that's signaled by their subscription callbacks and an "output" abort_source to push the combined request_abort further down the stack.
I don't want to solve the problem by making users duplicate glue code. But I also don't want a very wide API that has lots of options, resulting in something no-one remembers how it works. I'm looking for a simple an intuitive API, but we are in a bind because we allowed abort_source to become part of the data plane (in semaphore) and now we don't want to enlarge its footprint or slow it down. An intuitive (IMO) API would be to have an abort_source constructor that accepts a parent abort_source and subscribes to it. Perhaps we should discourage transient abort_source use in semaphore by deprecating it, and move the users to use a condition_variable instead. Long queues are better managed by custom code rather than overloading a semaphore. @denesb please share your experience here.
Why do you think those primitives are needed because of the way semaphore uses aborts_source?
We need to keep abort_source slim because semaphore uses it, and semaphore is a low-level primitive.
If we could declare that abort_source was a high-level primitive with just a few objects constructed and destroyed over the lifetime of the application, we could focus on its usability rather than its footprint.
Fair point. Again the original semaphore patch held a subscription not an entire abort_source. But I think we do need light wait abort source to allow higher granularity for things that can be aborted. Why not abort a speculative read for instance if it is no longer needed?
@bhalevy need it not related to the semaphore in any way. What if you want to abort an operation for one of two reasons: because Scylla is shutting down or because an administrator (or some internal logic) decided to abort only this particular operation? You have two abort_sources one subscribed to a stop another is for the operation you want to abort and you want to chain them. This is done manually today, but with a proper primitive it will become easier.
I want it to be done without an additional class, just an overload to abort_source constructor taking a higher scope abort_source. This was @bhalevy initial approach, which I liked, but then he switch to this one due to (real) concerns about increasing semaphore's footprint.
OK. But this one allows to have N number of sources chained which is nice.
abort_when_xx sounds like a function, not a class. I don't know how to feel about this. It looks like a maze of small features, for something that should be easy to use and understand. This is used on rare paths, so it shouldn't require a deep study to figure out.
This functionality could be composed by ad-hoc functions, e.g. for abort_when_either using two "input" abort sources, a condition_variable that's signaled by their subscription callbacks and an "output" abort_source to push the combined request_abort further down the stack.
I don't want to solve the problem by making users duplicate glue code. But I also don't want a very wide API that has lots of options, resulting in something no-one remembers how it works. I'm looking for a simple an intuitive API, but we are in a bind because we allowed abort_source to become part of the data plane (in semaphore) and now we don't want to enlarge its footprint or slow it down. An intuitive (IMO) API would be to have an abort_source constructor that accepts a parent abort_source and subscribes to it. Perhaps we should discourage transient abort_source use in semaphore by deprecating it, and move the users to use a condition_variable instead. Long queues are better managed by custom code rather than overloading a semaphore. @denesb please share your experience here.
Why do you think those primitives are needed because of the way semaphore uses aborts_source?
We need to keep abort_source slim because semaphore uses it, and semaphore is a low-level primitive. If we could declare that abort_source was a high-level primitive with just a few objects constructed and destroyed over the lifetime of the application, we could focus on its usability rather than its footprint.
Fair point. Again the original semaphore patch held a subscription not an entire abort_source. But I think we do need light wait abort source to allow higher granularity for things that can be aborted. Why not abort a speculative read for instance if it is no longer needed?
If we can have a nice-to-use interface AND always low footprint, there's no controversy.
But we can't have both. We can have fine-grained classes each adding a bit of functionality, or we can have a single easy to use class, but not both.
Should we use abort_source to abort individual requests, or something else? I don't want to add yet another thing, but maybe nothing new is needed.
@bhalevy need it not related to the semaphore in any way. What if you want to abort an operation for one of two reasons: because Scylla is shutting down or because an administrator (or some internal logic) decided to abort only this particular operation? You have two abort_sources one subscribed to a stop another is for the operation you want to abort and you want to chain them. This is done manually today, but with a proper primitive it will become easier.
I want it to be done without an additional class, just an overload to abort_source constructor taking a higher scope abort_source. This was @bhalevy initial approach, which I liked, but then he switch to this one due to (real) concerns about increasing semaphore's footprint.
OK. But this one allows to have N number of sources chained which is nice.
If we talk about introducing a scope, it's rarely needed.