You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@quickstep.apache.org by hbdeshmukh <gi...@git.apache.org> on 2017/11/23 17:09:49 UTC

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

GitHub user hbdeshmukh opened a pull request:

    https://github.com/apache/incubator-quickstep/pull/325

    DO NOT MERGE: Concurrent queries transactions

    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. 


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hbdeshmukh/incubator-quickstep concurrent-queries-transactions

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-quickstep/pull/325.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #325
    
----
commit 18692dbb6d6679aab89e503d388a5fc57889e811
Author: Harshad Deshmukh <hb...@apache.org>
Date:   2017-11-16T16:27:28Z

    Initial commit for APIs
    
    - 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.

commit 8f94489a8403c91110b454c7f93306d6c5d8fc57
Author: Harshad Deshmukh <hb...@apache.org>
Date:   2017-11-20T17:02:33Z

    Added more APIs to acquire locks
    
    - Locks can be on CatalogRelation as well as individual blocks.

----


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019995
  
    --- Diff: transaction/CompatibilityChecker.hpp ---
    @@ -0,0 +1,106 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +#define QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +
    +#include <utility>
    +#include <vector>
    +
    +#include "transaction/TransactionTable.hpp"
    +#include "utility/Macros.hpp"
    +
    +namespace quickstep {
    +
    +class CatalogRelation;
    +
    +namespace transaction {
    +
    +/**
    + * @brief A class that checks the compatibility of a transaction with the
    + *        other running transactions.
    + */
    +class CompatibilityChecker {
    + public:
    +  /**
    +   * @brief Constructor
    +   * @param transaction_table A lookup table that stores information about
    +   *        all the running and waiting transactions.
    +   */
    +  CompatibilityChecker(TransactionTable *transaction_table) {}
    --- End diff --
    
    Add `explicit`.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019935
  
    --- Diff: transaction/CMakeLists.txt ---
    @@ -50,10 +53,16 @@ add_library(quickstep_transaction_StronglyConnectedComponents
     add_library(quickstep_transaction_Transaction
                 ../empty_src.cpp
                 Transaction.hpp)
    +add_library(quickstep_transaction_CompatibilityChecker
    --- End diff --
    
    Alphabet order.
    
    Ditto below.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019903
  
    --- Diff: transaction/AdmissionControl.hpp ---
    @@ -0,0 +1,83 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +#define QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +
    +#include "utility/Macros.hpp"
    +#include "transaction/TransactionTable.hpp"
    +
    +namespace quickstep {
    +namespace transaction {
    +
    +class AdmissionControl {
    + public:
    +  /**
    +   * @brief Constructor
    +   * @param transaction_table A lookup table that stores information about
    +   *        all the running and waiting transactions.
    +   */
    +  AdmissionControl(TransactionTable *transaction_table) {}
    +
    +  virtual ~AdmissionControl() {}
    +
    +  /**
    +   * @brief Admit a transaction to the system.
    +   *
    +   * @note Check the transaction's compatibility with the other running
    +   *       transactions. If it is compatible, let it run, otherwise put
    +   *       the transaction in the waiting list.
    +   *
    +   * @note Accesses to the transaction_table may need protection.
    +   *
    +   * @param tid The ID of the given transaction.
    +   * @param resource_requests A vector of pairs such that each pair has a
    +   *        resource ID and its requested access mode.
    +   * @return True if the transaction can be admitted, false if it has to wait.
    +   */
    +  virtual bool admitTransaction(const transaction_id tid,
    +                                const std::vector<std::pair<ResourceId, AccessMode>> &resource_requests) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Attempt to admit a waiting transaction.
    +   *
    +   * @note Check the transaction's compatibility with the other running
    +   *       transactions. If it is compatible, let it run, otherwise put
    +   *       the transaction in the waiting list.
    +   *
    +   * @note Accesses to the transaction_table may need protection.
    +   *
    +   * @param tid The ID of the given transaction.
    +   * @return True if the transaction can be admitted, false if the
    +   *         transaction has to wait.
    +   */
    +  virtual bool admitWaitingTransaction(const transaction_id tid) {
    +    return false;
    +  }
    --- End diff --
    
    Optionally, I think it is better to mark these two APIs as `pure virtual` methods.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019502
  
    --- Diff: query_optimizer/QueryHandle.hpp ---
    @@ -178,6 +178,14 @@ class QueryHandle {
         query_result_relation_ = relation;
       }
     
    +  /**
    +   * @brief Return all the base relations referenced in this query.
    +   **/
    +  std::vector<relation_id> getAllReferencedBaseRelations() {
    --- End diff --
    
    This should mark as a `const` method, and I think we need to add a data member called `referenced_base_relations_`.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153020184
  
    --- Diff: transaction/CompatibilityChecker.hpp ---
    @@ -0,0 +1,106 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +#define QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +
    +#include <utility>
    +#include <vector>
    +
    +#include "transaction/TransactionTable.hpp"
    +#include "utility/Macros.hpp"
    +
    +namespace quickstep {
    +
    +class CatalogRelation;
    +
    +namespace transaction {
    +
    +/**
    + * @brief A class that checks the compatibility of a transaction with the
    + *        other running transactions.
    + */
    +class CompatibilityChecker {
    + public:
    +  /**
    +   * @brief Constructor
    +   * @param transaction_table A lookup table that stores information about
    +   *        all the running and waiting transactions.
    +   */
    +  CompatibilityChecker(TransactionTable *transaction_table) {}
    +
    +  virtual ~CompatibilityChecker() {}
    +
    +  /**
    +   * @brief Check if the given transaction is compatible with other
    +   *        running transactions.
    +   * @note The transaction table has the list of all the running or pending
    +   *       transactions. Lookup the table and check if the given transaction
    +   *       is compatible, based on the CC protocol specifications.
    +   * @note Accesses to the transaction_table may need protection.
    +
    +   * @param tid The ID of the given transaction
    +   * @param resource_requests A vector of pairs such that each pair has a
    +   *        resource ID and its requested access mode.
    +   * @return True if the transaction is compatible and false if it is not.
    +   */
    +  virtual bool isTranasctionCompatible(const transaction_id tid,
    +                                       const std::vector<std::pair<ResourceId, AccessMode>> &resource_requests) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Check if the given transaction can acquire a Catalog lock on the
    +   *        given relation.
    +   * @param tid The ID of the given transaction.
    +   * @param relation The given relation.
    +   * @param access_mode The access_mode requested to lock the catalog.
    +   * @return True if the lock can be acquired, false otherwise.
    +   */
    +  virtual bool canAcquireCatalogLock(const transaction_id tid,
    +                                     const CatalogRelation &relation,
    +                                     const AccessMode access_mode) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Check if the given transaction can acquire locks on all the blocks
    +   *        of the given relation.
    +   * @param tid The ID of the given transaction.
    +   * @param rid The ID of the given relation.
    +   * @param blocks The list of the blocks of the given relation.
    +   * @param access_mode The access_mode requested to lock the blocks.
    +   * @return True if the lock can be acquired, false otherwise.
    +   */
    +  virtual bool canAcquireBlockLocks(const transaction_id tid,
    +                                    const relation_id rid,
    +                                    const std::vector<block_id> &blocks,
    +                                    const AccessMode access_mode) {
    +    return false;
    +  }
    +
    + private:
    +  DISALLOW_COPY_AND_ASSIGN(CompatibilityChecker);
    +
    +};
    +
    +}  // namespace transaction
    +}  // namespace quickstep
    +
    +#endif  //QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    --- End diff --
    
    Add a whitespace after `//`, and add `EOL` in the end.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153020141
  
    --- Diff: transaction/CompatibilityChecker.hpp ---
    @@ -0,0 +1,106 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +#define QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +
    +#include <utility>
    +#include <vector>
    +
    +#include "transaction/TransactionTable.hpp"
    +#include "utility/Macros.hpp"
    +
    +namespace quickstep {
    +
    +class CatalogRelation;
    +
    +namespace transaction {
    +
    +/**
    + * @brief A class that checks the compatibility of a transaction with the
    + *        other running transactions.
    + */
    +class CompatibilityChecker {
    + public:
    +  /**
    +   * @brief Constructor
    +   * @param transaction_table A lookup table that stores information about
    +   *        all the running and waiting transactions.
    +   */
    +  CompatibilityChecker(TransactionTable *transaction_table) {}
    +
    +  virtual ~CompatibilityChecker() {}
    +
    +  /**
    +   * @brief Check if the given transaction is compatible with other
    +   *        running transactions.
    +   * @note The transaction table has the list of all the running or pending
    +   *       transactions. Lookup the table and check if the given transaction
    +   *       is compatible, based on the CC protocol specifications.
    +   * @note Accesses to the transaction_table may need protection.
    +
    +   * @param tid The ID of the given transaction
    +   * @param resource_requests A vector of pairs such that each pair has a
    +   *        resource ID and its requested access mode.
    +   * @return True if the transaction is compatible and false if it is not.
    +   */
    +  virtual bool isTranasctionCompatible(const transaction_id tid,
    +                                       const std::vector<std::pair<ResourceId, AccessMode>> &resource_requests) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Check if the given transaction can acquire a Catalog lock on the
    +   *        given relation.
    +   * @param tid The ID of the given transaction.
    +   * @param relation The given relation.
    +   * @param access_mode The access_mode requested to lock the catalog.
    +   * @return True if the lock can be acquired, false otherwise.
    +   */
    +  virtual bool canAcquireCatalogLock(const transaction_id tid,
    +                                     const CatalogRelation &relation,
    +                                     const AccessMode access_mode) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Check if the given transaction can acquire locks on all the blocks
    +   *        of the given relation.
    +   * @param tid The ID of the given transaction.
    +   * @param rid The ID of the given relation.
    +   * @param blocks The list of the blocks of the given relation.
    +   * @param access_mode The access_mode requested to lock the blocks.
    +   * @return True if the lock can be acquired, false otherwise.
    +   */
    +  virtual bool canAcquireBlockLocks(const transaction_id tid,
    +                                    const relation_id rid,
    +                                    const std::vector<block_id> &blocks,
    +                                    const AccessMode access_mode) {
    +    return false;
    +  }
    +
    + private:
    +  DISALLOW_COPY_AND_ASSIGN(CompatibilityChecker);
    +
    --- End diff --
    
    Remove this empty line.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019919
  
    --- Diff: transaction/AdmissionControl.hpp ---
    @@ -0,0 +1,83 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +#define QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +
    +#include "utility/Macros.hpp"
    +#include "transaction/TransactionTable.hpp"
    +
    +namespace quickstep {
    +namespace transaction {
    +
    +class AdmissionControl {
    + public:
    +  /**
    +   * @brief Constructor
    +   * @param transaction_table A lookup table that stores information about
    +   *        all the running and waiting transactions.
    +   */
    +  AdmissionControl(TransactionTable *transaction_table) {}
    +
    +  virtual ~AdmissionControl() {}
    +
    +  /**
    +   * @brief Admit a transaction to the system.
    +   *
    +   * @note Check the transaction's compatibility with the other running
    +   *       transactions. If it is compatible, let it run, otherwise put
    +   *       the transaction in the waiting list.
    +   *
    +   * @note Accesses to the transaction_table may need protection.
    +   *
    +   * @param tid The ID of the given transaction.
    +   * @param resource_requests A vector of pairs such that each pair has a
    +   *        resource ID and its requested access mode.
    +   * @return True if the transaction can be admitted, false if it has to wait.
    +   */
    +  virtual bool admitTransaction(const transaction_id tid,
    +                                const std::vector<std::pair<ResourceId, AccessMode>> &resource_requests) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Attempt to admit a waiting transaction.
    +   *
    +   * @note Check the transaction's compatibility with the other running
    +   *       transactions. If it is compatible, let it run, otherwise put
    +   *       the transaction in the waiting list.
    +   *
    +   * @note Accesses to the transaction_table may need protection.
    +   *
    +   * @param tid The ID of the given transaction.
    +   * @return True if the transaction can be admitted, false if the
    +   *         transaction has to wait.
    +   */
    +  virtual bool admitWaitingTransaction(const transaction_id tid) {
    +    return false;
    +  }
    +
    + private:
    +  DISALLOW_COPY_AND_ASSIGN(AdmissionControl);
    +};
    +
    +}  // namespace transaction
    +}  // namespace quickstep
    +
    +#endif  //QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    --- End diff --
    
    Add a whitespace after `//`.


---

[GitHub] incubator-quickstep issue #325: DO NOT MERGE: Concurrent queries transaction...

Posted by hbdeshmukh <gi...@git.apache.org>.
Github user hbdeshmukh commented on the issue:

    https://github.com/apache/incubator-quickstep/pull/325
  
    @pateljm @zuyu @jianqiao Can you take a look at these changes whenever possible? Thanks.


---

[GitHub] incubator-quickstep issue #325: DO NOT MERGE: Concurrent queries transaction...

Posted by jianqiao <gi...@git.apache.org>.
Github user jianqiao commented on the issue:

    https://github.com/apache/incubator-quickstep/pull/325
  
    The design looks good to me! It would be better to fast forward to subsequent PRs to see the actual usage.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019967
  
    --- Diff: transaction/CMakeLists.txt ---
    @@ -125,8 +138,8 @@ target_link_libraries(quickstep_transaction
                           quickstep_transaction_ResourceId
                           quickstep_transaction_StronglyConnectedComponents
                           quickstep_transaction_Transaction
    +                      quickstep_transaction_CompatibilityChecker
                           quickstep_transaction_TransactionTable)
    -
    --- End diff --
    
    Revert this change.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153020128
  
    --- Diff: transaction/CompatibilityChecker.hpp ---
    @@ -0,0 +1,106 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +#define QUICKSTEP_TRANSACTION_TRANSACTION_COMPATIBILITY_CHECKER_HPP_
    +
    +#include <utility>
    +#include <vector>
    +
    +#include "transaction/TransactionTable.hpp"
    +#include "utility/Macros.hpp"
    +
    +namespace quickstep {
    +
    +class CatalogRelation;
    +
    +namespace transaction {
    +
    +/**
    + * @brief A class that checks the compatibility of a transaction with the
    + *        other running transactions.
    + */
    +class CompatibilityChecker {
    + public:
    +  /**
    +   * @brief Constructor
    +   * @param transaction_table A lookup table that stores information about
    +   *        all the running and waiting transactions.
    +   */
    +  CompatibilityChecker(TransactionTable *transaction_table) {}
    +
    +  virtual ~CompatibilityChecker() {}
    +
    +  /**
    +   * @brief Check if the given transaction is compatible with other
    +   *        running transactions.
    +   * @note The transaction table has the list of all the running or pending
    +   *       transactions. Lookup the table and check if the given transaction
    +   *       is compatible, based on the CC protocol specifications.
    +   * @note Accesses to the transaction_table may need protection.
    +
    +   * @param tid The ID of the given transaction
    +   * @param resource_requests A vector of pairs such that each pair has a
    +   *        resource ID and its requested access mode.
    +   * @return True if the transaction is compatible and false if it is not.
    +   */
    +  virtual bool isTranasctionCompatible(const transaction_id tid,
    +                                       const std::vector<std::pair<ResourceId, AccessMode>> &resource_requests) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Check if the given transaction can acquire a Catalog lock on the
    +   *        given relation.
    +   * @param tid The ID of the given transaction.
    +   * @param relation The given relation.
    +   * @param access_mode The access_mode requested to lock the catalog.
    +   * @return True if the lock can be acquired, false otherwise.
    +   */
    +  virtual bool canAcquireCatalogLock(const transaction_id tid,
    +                                     const CatalogRelation &relation,
    +                                     const AccessMode access_mode) {
    +    return false;
    +  }
    +
    +  /**
    +   * @brief Check if the given transaction can acquire locks on all the blocks
    +   *        of the given relation.
    +   * @param tid The ID of the given transaction.
    +   * @param rid The ID of the given relation.
    +   * @param blocks The list of the blocks of the given relation.
    +   * @param access_mode The access_mode requested to lock the blocks.
    +   * @return True if the lock can be acquired, false otherwise.
    +   */
    +  virtual bool canAcquireBlockLocks(const transaction_id tid,
    +                                    const relation_id rid,
    +                                    const std::vector<block_id> &blocks,
    +                                    const AccessMode access_mode) {
    +    return false;
    +  }
    --- End diff --
    
    Optionally, mark as `pure virtual` methods.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019545
  
    --- Diff: transaction/AdmissionControl.hpp ---
    @@ -0,0 +1,83 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +#define QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +
    +#include "utility/Macros.hpp"
    +#include "transaction/TransactionTable.hpp"
    --- End diff --
    
    Alphabet order.


---

[GitHub] incubator-quickstep pull request #325: DO NOT MERGE: Concurrent queries tran...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/325#discussion_r153019788
  
    --- Diff: transaction/AdmissionControl.hpp ---
    @@ -0,0 +1,83 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +#define QUICKSTEP_TRANSACTION_ADMISSION_CONTROL_HPP_
    +
    +#include "utility/Macros.hpp"
    +#include "transaction/TransactionTable.hpp"
    +
    +namespace quickstep {
    +namespace transaction {
    +
    +class AdmissionControl {
    + public:
    +  /**
    +   * @brief Constructor
    +   * @param transaction_table A lookup table that stores information about
    +   *        all the running and waiting transactions.
    +   */
    +  AdmissionControl(TransactionTable *transaction_table) {}
    --- End diff --
    
    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?


---