[GitHub] [incubator-heron] huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr

2020-03-18 Thread GitBox 
huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr
URL: https://github.com/apache/incubator-heron/pull/3492#discussion_r394498044
 
 

 ##
 File path: heron/stmgr/src/cpp/manager/stmgr-clientmgr.cpp
 ##
 @@ -115,7 +115,7 @@ bool StMgrClientMgr::DidAnnounceBackPressure() {
   return stream_manager_->DidAnnounceBackPressure();
 }
 
-shared_ptr StMgrClientMgr::CreateClient(const sp_string& 
_other_stmgr_id,
+StMgrClient* StMgrClientMgr::CreateClient(const sp_string& _other_stmgr_id,
 
 Review comment:
   when a new physical plan is issued from tmaster, the stmgr-client-manager 
swaps the old stmgr out from array and explicitly calls quit() destructor.
   btw, this pr just reverts it to v0.20.0, those lines have not been changed 
till v0.20.1 for 4 years and we did not see any issue during those 4 years. 
thus believe it is safe


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-heron] huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr

2020-03-18 Thread GitBox 
huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr
URL: https://github.com/apache/incubator-heron/pull/3492#discussion_r394498044
 
 

 ##
 File path: heron/stmgr/src/cpp/manager/stmgr-clientmgr.cpp
 ##
 @@ -115,7 +115,7 @@ bool StMgrClientMgr::DidAnnounceBackPressure() {
   return stream_manager_->DidAnnounceBackPressure();
 }
 
-shared_ptr StMgrClientMgr::CreateClient(const sp_string& 
_other_stmgr_id,
+StMgrClient* StMgrClientMgr::CreateClient(const sp_string& _other_stmgr_id,
 
 Review comment:
   when a new physical plan is issued from tmaster, the stmgr-client-manager 
swaps the old stmgr out from array and explicitly calls quit() destructor.
   btw, this pr just reverts it to v0.20.0, those lines have not been changed 
till v0.20.0 for 4 years and we did not see any issue during those 4 years. 
thus believe it is safe


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-heron] huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr

2020-03-18 Thread GitBox 
huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr
URL: https://github.com/apache/incubator-heron/pull/3492#discussion_r394498044
 
 

 ##
 File path: heron/stmgr/src/cpp/manager/stmgr-clientmgr.cpp
 ##
 @@ -115,7 +115,7 @@ bool StMgrClientMgr::DidAnnounceBackPressure() {
   return stream_manager_->DidAnnounceBackPressure();
 }
 
-shared_ptr StMgrClientMgr::CreateClient(const sp_string& 
_other_stmgr_id,
+StMgrClient* StMgrClientMgr::CreateClient(const sp_string& _other_stmgr_id,
 
 Review comment:
   when a new physical plan is issued from tmaster, the stmgr-client-manager 
swaps the old stmgr out from array and explicitly calls quit() destructor.
   btw, this pr just reverts it to v0.20.0, those lines have not been changed 
till v0.20.0 for 4 years and we did not see any issue during those 4 years. 
believe it is safe


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-heron] huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr

2020-03-18 Thread GitBox 
huijunwu commented on a change in pull request #3492: revert the stmgr clientmgr
URL: https://github.com/apache/incubator-heron/pull/3492#discussion_r394498044
 
 

 ##
 File path: heron/stmgr/src/cpp/manager/stmgr-clientmgr.cpp
 ##
 @@ -115,7 +115,7 @@ bool StMgrClientMgr::DidAnnounceBackPressure() {
   return stream_manager_->DidAnnounceBackPressure();
 }
 
-shared_ptr StMgrClientMgr::CreateClient(const sp_string& 
_other_stmgr_id,
+StMgrClient* StMgrClientMgr::CreateClient(const sp_string& _other_stmgr_id,
 
 Review comment:
   when a new physical plan is issued from tmaster, the stmgr-client-manager 
swaps the old stmgr out from array and explicitly calls quit() destructor.
   btw, this pr just reverts it to v0.20.0, those lines have not been changed 
till v0.20.0 for 4 years and we did not see any issue during those 4 years


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services