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

Make FactorizedTable threadsafe (especially the merge function) #4204

Open
semihsalihoglu-uw opened this issue Sep 6, 2024 · 0 comments
Open
Assignees
Labels
good-warm-up Good warm up feature refactoring

Comments

@semihsalihoglu-uw
Copy link
Contributor

Currently FactorizedTable is not a threadsafe class. In particular it has a merge function to merge multiple FactorizedTables that delegates the responsibility of thread safety, i.e., acquiring the locks, to the caller. This makes the code harder to read as one needs to figure out the call stack to find whether or not a lock has been acquired. It also leads to duplicated lock acquisition code in cases when the only reason the caller uses a mutex and lock is to ensure FactorizedTable::merge's safety (which is currently what happens on ResultCollectorSharedState::mergeLocalTable and the GDS RJOutputWriterVC code in my branch).

Instead FactorizedTable should be threadsafe itself by keeping an internal lock. Even if an outside lock is acquired this is fine and in some cases, as in RJOutputWriterVC, this can help us remove the duplicated locking code from the caller. So we should refactor this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-warm-up Good warm up feature refactoring
Projects
None yet
Development

No branches or pull requests

2 participants