You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@quickstep.apache.org by zuyu <gi...@git.apache.org> on 2016/07/22 18:32:27 UTC

[GitHub] incubator-quickstep pull request #63: Introduced PolicyEnforcer implementati...

GitHub user zuyu opened a pull request:

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

    Introduced PolicyEnforcer implementation for the distributed version.

    

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

    $ git pull https://github.com/apache/incubator-quickstep policy-enforcer-dist

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

    https://github.com/apache/incubator-quickstep/pull/63.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 #63
    
----
commit 03b29a118741d0582e49f1bdcfd228048273c62f
Author: Zuyu Zhang <zu...@twitter.com>
Date:   2016-07-22T18:31:33Z

    Introduced PolicyEnforcer implementation for the distributed version.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #63: Introduced PolicyEnforcer implementati...

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

    https://github.com/apache/incubator-quickstep/pull/63#discussion_r72646779
  
    --- Diff: query_execution/PolicyEnforcerDistributed.hpp ---
    @@ -0,0 +1,98 @@
    +/**
    + *   Licensed 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_QUERY_EXECUTION_POLICY_ENFORCER_DISTRIBUTED_HPP_
    +#define QUICKSTEP_QUERY_EXECUTION_POLICY_ENFORCER_DISTRIBUTED_HPP_
    +
    +#include <cstddef>
    +#include <memory>
    +#include <vector>
    +
    +#include "query_execution/PolicyEnforcerBase.hpp"
    +#include "query_execution/ShiftbossDirectory.hpp"
    +#include "utility/Macros.hpp"
    +
    +#include "tmb/id_typedefs.h"
    +
    +namespace tmb { class MessageBus; }
    +
    +namespace quickstep {
    +
    +class CatalogDatabaseLite;
    +class QueryHandle;
    +
    +namespace serialization { class WorkOrderMessage; }
    +
    +/** \addtogroup QueryExecution
    + *  @{
    + */
    +
    +/**
    + * @brief A class that ensures that a high level policy is maintained
    + *        in sharing resources among concurrent queries.
    + **/
    +class PolicyEnforcerDistributed final : public PolicyEnforcerBase {
    + public:
    +  /**
    +   * @brief Constructor.
    +   *
    +   * @param foreman_client_id The TMB client ID of the Foreman.
    +   * @param catalog_database The CatalogDatabase used.
    +   * @param bus The TMB.
    --- End diff --
    
    Please sync the doxygen. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep issue #63: Introduced PolicyEnforcer implementation for ...

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

    https://github.com/apache/incubator-quickstep/pull/63
  
    Looks good to me, except few minor comments. 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #63: DO NOT MERGE: Introduced PolicyEnforce...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu closed the pull request at:

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


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #63: Introduced PolicyEnforcer implementati...

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

    https://github.com/apache/incubator-quickstep/pull/63#discussion_r72647028
  
    --- Diff: query_execution/PolicyEnforcerDistributed.cpp ---
    @@ -0,0 +1,109 @@
    +/**
    + *   Licensed 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.
    + **/
    +
    +#include "query_execution/PolicyEnforcerDistributed.hpp"
    +
    +#include <cstddef>
    +#include <memory>
    +#include <queue>
    +#include <utility>
    +#include <unordered_map>
    +#include <vector>
    +
    +#include "query_execution/QueryExecutionMessages.pb.h"
    +#include "query_execution/QueryExecutionState.hpp"
    +#include "query_execution/QueryManagerBase.hpp"
    +#include "query_execution/QueryManagerDistributed.hpp"
    +#include "query_optimizer/QueryHandle.hpp"
    +
    +#include "gflags/gflags.h"
    +#include "glog/logging.h"
    +
    +using std::unique_ptr;
    +using std::vector;
    +
    +namespace quickstep {
    +
    +namespace S = serialization;
    +
    +DEFINE_uint64(max_msgs_per_dispatch_round, 20, "Maximum number of messages that"
    +              " can be allocated in a single round of dispatch of messages to"
    +              " the workers.");
    --- End diff --
    
    Should the description say "dispatch of messages to the shiftbosses" instead of "workers"?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---