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

Support thread group #51

Open
BusyJay opened this issue Mar 26, 2020 · 5 comments · May be fixed by #55
Open

Support thread group #51

BusyJay opened this issue Mar 26, 2020 · 5 comments · May be fixed by #55
Assignees

Comments

@BusyJay
Copy link
Member

BusyJay commented Mar 26, 2020

Is your feature request related to a problem? Please describe.

fail-rs utilizes global registry to expose simple APIs and convenient FailPoint definition. But it also means all parallel tests have to be run one by one and do cleanup between each run to avoid configurations affect each other.

Describe the solution you'd like

This issue proposes to utilize thread group. Each test case defines a unique thread group, all configuration will be bound to exact one thread group. Every time a new thread is spawn, it needs to be registered to one thread group to make FailPoint reads configurations. If a thread is not registered to any group, it belongs to a default global group.

New public APIs include:

pub fn current_fail_group() -> FailGroup;

impl FailGroup {
    pub fn register_current(&self) -> Result<()>;
    pub fn deregister_current(&self);
}

Note that it doesn't require users have the ability to spawn a thread, register the thread before using FailPoint is enough.

Describe alternatives you've considered

One solution to this is pass the global registry to struct constructor, but it will interfere the general code heavily, it needs to be passed to anywhere FailPoints are defined.

Another solution is #24, but it lacks threaded cases support.

@BusyJay
Copy link
Member Author

BusyJay commented Mar 26, 2020

/cc @brson @Little-Wallace @lucab

@lucab
Copy link
Member

lucab commented Mar 27, 2020

@BusyJay nice! A bunch of high-level questions:

  • can the thread-group logic be folded into FailScenario? register_current doesn't seem to be very different from threadgroup-bound setup.
  • would it be better for the API to take a ThreadId argument instead?
  • I have no idea how this would actually work at the low level; how do you intend to plug this into the eval logic?

@BusyJay
Copy link
Member Author

BusyJay commented Mar 27, 2020

can the thread-group logic be folded into FailScenario?

FailGroup can be accessed via a global fuction, so it may not be appropriate to use FailScenario directly. But I think it can be defined as a field of FailScenario if necessary.

would it be better for the API to take a ThreadId argument instead?

I'm not sure. I find it more convenient to just register current thread instead of others. For example, thread pool generally provides after_start, before_stop hooks to allow do work in a thread, but generally won't provide thread ID directly.

plug this into the eval logic?

When evaluating, it uses current thread ID to find out which thread group it belongs to, and then check the rules of the group.

@Little-Wallace
Copy link
Contributor

It seems complex. Why can not we launch multiple process and each process runs some test with a single thread?

@BusyJay
Copy link
Member Author

BusyJay commented Mar 28, 2020

Process isn't compatible with the default test runner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants