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

yatp could not preempt threads #71

Open
skyzh opened this issue Dec 2, 2020 · 6 comments
Open

yatp could not preempt threads #71

skyzh opened this issue Dec 2, 2020 · 6 comments

Comments

@skyzh
Copy link
Member

skyzh commented Dec 2, 2020

After introducing MVCC and txn, we now have 3 long-running background threads.

  • memtable flush
  • batching write request
  • compaction * 3

If we put all threads in one thread pool, one of them won't even begin to execute in GitHub Action. Current workaround is to use std::thread for flush and write, and pool for compaction. We may later investigate how to integrate all threads into a thread pool.

@skyzh
Copy link
Member Author

skyzh commented Dec 2, 2020

Seems that this is how thread pool is expected to work. It's more appropriate to use std::thread in agatedb, as we need long running threads instead of spawning short-running tasks.

@skyzh skyzh closed this as completed Dec 2, 2020
@BusyJay
Copy link
Member

BusyJay commented Dec 2, 2020

Can you explain more about "how thread pool is expected to work"? AgateDB should allow sharing global thread pool with other system.

@BusyJay BusyJay reopened this Dec 2, 2020
@skyzh
Copy link
Member Author

skyzh commented Dec 2, 2020

From yatp source code, it by default spawns a maximum of num_cpu threads. https://github.com/tikv/yatp/blob/master/src/pool/builder.rs#L37 Therefore, (from my understanding, if threads in thread pool won't be preempted), there would have at most num_cpu concurrently running threads in the background.

In agatedb with default configuration, we would have at least 5 concurrent tasks running in the background. If other systems have their thread pool misconfigured (like using num_cpus as maximum thread, for GitHub Action, it should be 2), then the database would easily go into stall state (e.g. compaction is never run).

@BusyJay
Copy link
Member

BusyJay commented Dec 2, 2020

How about setting a larger capacity?

@skyzh
Copy link
Member Author

skyzh commented Dec 2, 2020

How about setting a larger capacity?

If we have 5 threads, and the thread pool always has these 5 threads running, there seems no difference with std::thread :(

Anyway, I have put all threads back into yatp, and set max_threads for the pool. adf14f7

@BusyJay
Copy link
Member

BusyJay commented Dec 2, 2020

Setting a larger capacity is to get around the CI issues. In fact, the thread pool should not be created by the library, it should be a builder parameter given by users. It is critical in resource limited environment to have more precise control on threads.

@skyzh skyzh changed the title yatp could not correctly yield threads yatp could not preempt threads Dec 2, 2020
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

No branches or pull requests

2 participants