[GitHub] incubator-quickstep pull request #355: QUICKSTEP-127 Data provider thread

[asfgit]

Github user asfgit closed the pull request at:

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


---

[GitHub] incubator-quickstep pull request #355: QUICKSTEP-127 Data provider thread

[zuyu]

Github user zuyu commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/355#discussion_r192827208
  
--- Diff: storage/CMakeLists.txt ---
@@ -120,6 +120,11 @@ configure_file (
 QS_PROTOBUF_GENERATE_CPP(storage_AggregationOperationState_proto_srcs
  storage_AggregationOperationState_proto_hdrs
  AggregationOperationState.proto)
+if (ENABLE_NETWORK_CLI)
+QS_PROTOBUF_GENERATE_CPP(storage_BlockWire_proto_srcs
--- End diff --

Two whitespace indentation, please.

Similarly below.


---

[GitHub] incubator-quickstep pull request #355: QUICKSTEP-127 Data provider thread

[hbdeshmukh]

Github user hbdeshmukh commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/355#discussion_r190785380
  
--- Diff: storage/DataProviderThread.hpp ---
@@ -0,0 +1,103 @@
+/**
+ * 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_STORAGE_DATA_PROVIDER_THREAD_HPP_
+#define QUICKSTEP_STORAGE_DATA_PROVIDER_THREAD_HPP_
+
+#include 
+
+#include "query_execution/QueryExecutionTypedefs.hpp"
+#include "storage/StorageConfig.h"
+#include "threading/Thread.hpp"
+#include "utility/Macros.hpp"
+
+#include "tmb/native_net_client_message_bus.h"
+
+namespace quickstep {
+
+class CatalogDatabase;
+class CatalogRelation;
+class QueryProcessor;
+class StorageManager;
+
+/** \addtogroup Storage
+ *  @{
+ */
+
+/**
+ * @brief A thread that provides access to query results (e.g. storage 
blocks),
+ *to Quickstep clients.
--- End diff --

This thread will be responsible to drop temporary tables created by 
queries. To perform this drop operation, it needs access to both 
CatalogDatabase and QueryProcessor. Details in the code 
[here](https://github.com/hbdeshmukh/incubator-quickstep/blob/6967ca57c448a17b63e176bf8bc1576066fe893a/storage/DataProviderThread.cpp#L79)


---

[GitHub] incubator-quickstep pull request #355: QUICKSTEP-127 Data provider thread

[hbdeshmukh]

Github user hbdeshmukh commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/355#discussion_r190785209
  
--- Diff: storage/DataProviderThread.hpp ---
@@ -0,0 +1,103 @@
+/**
+ * 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_STORAGE_DATA_PROVIDER_THREAD_HPP_
+#define QUICKSTEP_STORAGE_DATA_PROVIDER_THREAD_HPP_
+
+#include 
+
+#include "query_execution/QueryExecutionTypedefs.hpp"
+#include "storage/StorageConfig.h"
+#include "threading/Thread.hpp"
+#include "utility/Macros.hpp"
+
+#include "tmb/native_net_client_message_bus.h"
+
+namespace quickstep {
+
+class CatalogDatabase;
+class CatalogRelation;
+class QueryProcessor;
+class StorageManager;
+
+/** \addtogroup Storage
+ *  @{
+ */
+
+/**
+ * @brief A thread that provides access to query results (e.g. storage 
blocks),
+ *to Quickstep clients.
--- End diff --

The DataProviderThread class will be part of the Quickstep server process. 
The following image describes the role of this thread. 

![image](https://user-images.githubusercontent.com/3750801/40525388-5e6225da-5fa5-11e8-818c-fc6f913f471c.png)



---

[GitHub] incubator-quickstep pull request #355: QUICKSTEP-127 Data provider thread

[zuyu]

Github user zuyu commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/355#discussion_r189664042
  
--- Diff: storage/DataProviderThread.hpp ---
@@ -0,0 +1,103 @@
+/**
+ * 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_STORAGE_DATA_PROVIDER_THREAD_HPP_
+#define QUICKSTEP_STORAGE_DATA_PROVIDER_THREAD_HPP_
+
+#include 
+
+#include "query_execution/QueryExecutionTypedefs.hpp"
+#include "storage/StorageConfig.h"
+#include "threading/Thread.hpp"
+#include "utility/Macros.hpp"
+
+#include "tmb/native_net_client_message_bus.h"
+
+namespace quickstep {
+
+class CatalogDatabase;
+class CatalogRelation;
+class QueryProcessor;
+class StorageManager;
+
+/** \addtogroup Storage
+ *  @{
+ */
+
+/**
+ * @brief A thread that provides access to query results (e.g. storage 
blocks),
+ *to Quickstep clients.
--- End diff --

Could you please provide more info regarding this class, including which 
class will own this class, and why it needs write permissions to both 
`CatalogDatabase` and `QueryProcessor`? Thanks!


---

[GitHub] incubator-quickstep pull request #355: QUICKSTEP-127 Data provider thread

[hbdeshmukh]

GitHub user hbdeshmukh opened a pull request:

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

QUICKSTEP-127 Data provider thread 

- Thread used for receiving block requests from clients and sending the
contents of a block back to the client.

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

$ git pull https://github.com/hbdeshmukh/incubator-quickstep 
data-provider-thread

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

https://github.com/apache/incubator-quickstep/pull/355.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 #355


commit 6967ca57c448a17b63e176bf8bc1576066fe893a
Author: Harshad Deshmukh 
Date:   2018-04-06T21:00:54Z

Data Provider thread added.

- Thread used for receiving block requests from clients and sending the
contents of a block back to the client.




---