DO NOT MERGE: Concurrent queries transactions#325
DO NOT MERGE: Concurrent queries transactions#325hbdeshmukh wants to merge 4 commits intoapache:masterfrom
Conversation
zuyu
left a comment
zuyu
left a comment
There was a problem hiding this comment.
I have done one pass.
Also, I hope there will be some follow-up PRs regarding how to use these two new base classes, as it seems incomplete to me even for the API design, let alone the integration to the query optimizer and the execution engine.
query_optimizer/QueryHandle.hpp
Outdated
| /** | ||
| * @brief Return all the base relations referenced in this query. | ||
| **/ | ||
| std::vector<relation_id> getAllReferencedBaseRelations() { |
There was a problem hiding this comment.
This should mark as a const method, and I think we need to add a data member called referenced_base_relations_.
| #define QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_ | ||
|
|
||
| #include "utility/Macros.hpp" | ||
| #include "transaction/TransactionTable.hpp" |
transaction/AdmissionControl.hpp
Outdated
| * @param transaction_table A lookup table that stores information about | ||
| * all the running and waiting transactions. | ||
| */ | ||
| AdmissionControl(TransactionTable *transaction_table) {} |
There was a problem hiding this comment.
Add explicit.
I was wondering which object will own transaction_table. And does this class need to store the pointer?
Also, should this class contain the CompatibilityChecker object?
| */ | ||
| virtual bool admitWaitingTransaction(const transaction_id tid) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Optionally, I think it is better to mark these two APIs as pure virtual methods.
transaction/AdmissionControl.hpp
Outdated
| } // namespace transaction | ||
| } // namespace quickstep | ||
|
|
||
| #endif //QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_ |
| quickstep_transaction_Transaction | ||
| quickstep_transaction_CompatibilityChecker | ||
| quickstep_transaction_TransactionTable) | ||
|
|
transaction/CompatibilityChecker.hpp
Outdated
| * @param transaction_table A lookup table that stores information about | ||
| * all the running and waiting transactions. | ||
| */ | ||
| CompatibilityChecker(TransactionTable *transaction_table) {} |
| const std::vector<block_id> &blocks, | ||
| const AccessMode access_mode) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Optionally, mark as pure virtual methods.
transaction/CompatibilityChecker.hpp
Outdated
|
|
||
| private: | ||
| DISALLOW_COPY_AND_ASSIGN(CompatibilityChecker); | ||
|
|
transaction/CompatibilityChecker.hpp
Outdated
| } // namespace transaction | ||
| } // namespace quickstep | ||
|
|
||
| #endif //QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_ No newline at end of file |
There was a problem hiding this comment.
Add a whitespace after //, and add EOL in the end.
|
The design looks good to me! It would be better to fast forward to subsequent PRs to see the actual usage. |
- CompatabilityChecker base class in transaction checks the compatability of an incoming transaction with other running transactions. - AdmissionControl base class decides whether to admit or waitlist an incoming transaction. - QueryHandle class provides a method to list all the base relations referenced in a transaction.
- Locks can be on CatalogRelation as well as individual blocks.
hbdeshmukh
force-pushed
the
concurrent-queries-transactions
branch
from
8f94489 to
a5eccaf
Compare
- Locks entire database in an exclusive lock for any query. - Rules for finding referenced base relations for an input logical plan. - Changes in PolicyEnforcer to admit queries from the transaction's admission control.
hbdeshmukh
force-pushed
the
concurrent-queries-transactions
branch
from
a5eccaf to
2ed20c1
Compare
3 participants
Hello,
This PR has some skeleton code for basic transactional queries support in quickstep. This PR follows the design proposed in the presentation attached with https://issues.apache.org/jira/browse/QUICKSTEP-107.