Github user zuyu commented on the issue:
https://github.com/apache/incubator-quickstep/pull/219
Hi @hbdeshmukh,
Most of the newly added methods could be marked as `const`, and I'll merge
this PR after you make the following suggested changes.
Btw, this could be a follow-up PR. What do you think if we instead add some
new TMB messages to get the memory usages in `QueryContext` so that both the
single node and the distributed version could use this function? Specifically,
for the single node, both the sender and receiver is `ForemanSingleNode`, but
in the distributed node, the sender is `ForemanDistributed`, while the receiver
is `Shiftboss`.
```
diff --git a/query_execution/QueryContext.cpp
b/query_execution/QueryContext.cpp
index b35ab0a..8ba77ab 100644
--- a/query_execution/QueryContext.cpp
+++ b/query_execution/QueryContext.cpp
@@ -269,7 +269,7 @@ bool QueryContext::ProtoIsValid(const
serialization::QueryContext &proto,
return proto.IsInitialized();
}
-std::size_t QueryContext::getJoinHashTablesMemoryBytes() {
+std::size_t QueryContext::getJoinHashTablesMemoryBytes() const {
SpinSharedMutexSharedLock<false> lock(hash_tables_mutex_);
std::size_t memory = 0;
for (std::size_t hashtable_id = 0;
@@ -287,7 +287,7 @@ std::size_t
QueryContext::getJoinHashTablesMemoryBytes() {
return memory;
}
-std::size_t QueryContext::getAggregationStatesMemoryBytes() {
+std::size_t QueryContext::getAggregationStatesMemoryBytes() const {
SpinSharedMutexSharedLock<false> lock(aggregation_states_mutex_);
std::size_t memory = 0;
for (std::size_t agg_state_id = 0;
@@ -301,7 +301,7 @@ std::size_t
QueryContext::getAggregationStatesMemoryBytes() {
}
void QueryContext::getTempRelationIDs(
- std::vector<relation_id> *temp_relation_ids) {
+ std::vector<relation_id> *temp_relation_ids) const {
SpinSharedMutexSharedLock<false> lock(insert_destinations_mutex_);
DCHECK(temp_relation_ids != nullptr);
for (std::size_t id = 0; id < insert_destinations_.size(); ++id) {
diff --git a/query_execution/QueryContext.hpp
b/query_execution/QueryContext.hpp
index f3419cb..ebc9506 100644
--- a/query_execution/QueryContext.hpp
+++ b/query_execution/QueryContext.hpp
@@ -569,7 +569,7 @@ class QueryContext {
* used for query execution (e.g. join hash tables, aggregation
hash
* tables) in bytes.
**/
- std::size_t getTempStructuresMemoryBytes() {
+ std::size_t getTempStructuresMemoryBytes() const {
return getJoinHashTablesMemoryBytes() +
getAggregationStatesMemoryBytes();
}
@@ -577,13 +577,13 @@ class QueryContext {
* @brief Get the total memory footprint in bytes of the join hash tables
* used for query execution.
**/
- std::size_t getJoinHashTablesMemoryBytes();
+ std::size_t getJoinHashTablesMemoryBytes() const;
/**
* @brief Get the total memory footprint in bytes of the aggregation hash
* tables used for query execution.
**/
- std::size_t getAggregationStatesMemoryBytes();
+ std::size_t getAggregationStatesMemoryBytes() const;
/**
* @brief Get the list of IDs of temporary relations in this query.
@@ -591,7 +591,7 @@ class QueryContext {
* @param temp_relation_ids A pointer to the vector that will store the
* relation IDs.
**/
- void getTempRelationIDs(std::vector<relation_id> *temp_relation_ids);
+ void getTempRelationIDs(std::vector<relation_id> *temp_relation_ids)
const;
private:
/**
diff --git a/query_execution/QueryManagerBase.hpp
b/query_execution/QueryManagerBase.hpp
index 01a65b4..b6ed247 100644
--- a/query_execution/QueryManagerBase.hpp
+++ b/query_execution/QueryManagerBase.hpp
@@ -276,7 +276,7 @@ class QueryManagerBase {
*
* @note This method returns a best guess consumption, at the time of
the call.
**/
- virtual std::size_t getQueryMemoryConsumptionBytes() {
+ virtual std::size_t getQueryMemoryConsumptionBytes() const {
return 0;
}
diff --git a/query_execution/QueryManagerSingleNode.cpp
b/query_execution/QueryManagerSingleNode.cpp
index c4897df..e3f349f 100644
--- a/query_execution/QueryManagerSingleNode.cpp
+++ b/query_execution/QueryManagerSingleNode.cpp
@@ -195,7 +195,7 @@ void QueryManagerSingleNode::getRebuildWorkOrders(const
dag_node_index index,
}
}
-std::size_t QueryManagerSingleNode::getQueryMemoryConsumptionBytes() {
+std::size_t QueryManagerSingleNode::getQueryMemoryConsumptionBytes() const
{
const std::size_t temp_relations_memory =
getTotalTempRelationMemoryInBytes();
const std::size_t temp_data_structures_memory =
@@ -203,7 +203,7 @@ std::size_t
QueryManagerSingleNode::getQueryMemoryConsumptionBytes() {
return temp_relations_memory + temp_data_structures_memory;
}
-std::size_t QueryManagerSingleNode::getTotalTempRelationMemoryInBytes() {
+std::size_t QueryManagerSingleNode::getTotalTempRelationMemoryInBytes()
const {
std::vector<relation_id> temp_relation_ids;
query_context_->getTempRelationIDs(&temp_relation_ids);
std::size_t memory = 0;
diff --git a/query_execution/QueryManagerSingleNode.hpp
b/query_execution/QueryManagerSingleNode.hpp
index daf4d54..6c5e38e 100644
--- a/query_execution/QueryManagerSingleNode.hpp
+++ b/query_execution/QueryManagerSingleNode.hpp
@@ -95,7 +95,7 @@ class QueryManagerSingleNode final : public
QueryManagerBase {
return query_context_.get();
}
- std::size_t getQueryMemoryConsumptionBytes() override;
+ std::size_t getQueryMemoryConsumptionBytes() const override;
private:
bool checkNormalExecutionOver(const dag_node_index index) const override
{
@@ -130,7 +130,7 @@ class QueryManagerSingleNode final : public
QueryManagerBase {
* @brief Get the total memory (in bytes) occupied by temporary relations
* created during the query execution.
**/
- std::size_t getTotalTempRelationMemoryInBytes();
+ std::size_t getTotalTempRelationMemoryInBytes() const;
const tmb::client_id foreman_client_id_;
diff --git a/storage/AggregationOperationState.cpp
b/storage/AggregationOperationState.cpp
index fe70d5d..9388bdb 100644
--- a/storage/AggregationOperationState.cpp
+++ b/storage/AggregationOperationState.cpp
@@ -946,7 +946,7 @@ void
AggregationOperationState::finalizeHashTableImplThreadPrivate(
output_destination->bulkInsertTuples(&complete_result);
}
-std::size_t AggregationOperationState::getMemoryConsumptionBytes() {
+std::size_t AggregationOperationState::getMemoryConsumptionBytes() const {
std::size_t memory =
getMemoryConsumptionBytesHelper(distinctify_hashtables_);
memory += getMemoryConsumptionBytesHelper(group_by_hashtables_);
memory += collision_free_hashtable_->getMemoryConsumptionBytes();
@@ -957,7 +957,7 @@ std::size_t
AggregationOperationState::getMemoryConsumptionBytes() {
std::size_t AggregationOperationState::getMemoryConsumptionBytesHelper(
const std::vector<std::unique_ptr<AggregationStateHashTableBase>>
- &hashtables) {
+ &hashtables) const {
std::size_t memory = 0;
for (std::size_t ht_id = 0; ht_id < hashtables.size(); ++ht_id) {
if (hashtables[ht_id] != nullptr) {
diff --git a/storage/AggregationOperationState.hpp
b/storage/AggregationOperationState.hpp
index 7a0f0b8..e6af494 100644
--- a/storage/AggregationOperationState.hpp
+++ b/storage/AggregationOperationState.hpp
@@ -210,7 +210,7 @@ class AggregationOperationState {
/**
* @brief Get the memory footprint of the AggregationOperationState.
**/
- std::size_t getMemoryConsumptionBytes();
+ std::size_t getMemoryConsumptionBytes() const;
private:
// Check whether partitioned aggregation can be applied.
@@ -260,7 +260,7 @@ class AggregationOperationState {
std::size_t getMemoryConsumptionBytesHelper(
**/
- std::size_t getMemoryConsumptionBytes();
+ std::size_t getMemoryConsumptionBytes() const;
private:
// Check whether partitioned aggregation can be applied.
@@ -260,7 +260,7 @@ class AggregationOperationState {
std::size_t getMemoryConsumptionBytesHelper(
const std::vector<std::unique_ptr<AggregationStateHashTableBase>>
- &hashtables);
+ &hashtables) const;
// Common state for all aggregates in this operation: the input
relation, the
// filter predicate (if any), and the list of GROUP BY expressions (if
any).
diff --git a/storage/HashTablePool.hpp b/storage/HashTablePool.hpp
index 07d98ed..6dbd7f9 100644
--- a/storage/HashTablePool.hpp
+++ b/storage/HashTablePool.hpp
@@ -126,7 +126,7 @@ class HashTablePool {
/**
* @brief Get the total memory consumed by the hash tables in this pool.
**/
- std::size_t getMemoryConsumptionPoolBytes() {
+ std::size_t getMemoryConsumptionPoolBytes() const {
std::size_t memory = 0;
for (std::size_t ht_id = 0; ht_id < hash_tables_.size(); ++ht_id) {
if (hash_tables_[ht_id] != nullptr) {
diff --git a/storage/PartitionedHashTablePool.hpp
b/storage/PartitionedHashTablePool.hpp
index 234cdfd..bfe8b8b 100644
--- a/storage/PartitionedHashTablePool.hpp
+++ b/storage/PartitionedHashTablePool.hpp
@@ -116,7 +116,7 @@ class PartitionedHashTablePool {
/**
* @brief Get the total memory consumed by the hash tables in this pool.
**/
- std::size_t getMemoryConsumptionPoolBytes() {
+ std::size_t getMemoryConsumptionPoolBytes() const {
std::size_t memory = 0;
for (std::size_t ht_id = 0; ht_id < hash_tables_.size(); ++ht_id) {
if (hash_tables_[ht_id] != nullptr) {
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
