You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/06/16 03:35:56 UTC

[GitHub] [incubator-doris] chaoyli opened a new pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

chaoyli opened a new pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883


   Now some SHOW PROC statementss have no message when executed
   in follower. These message are only in master. So I add
   FORWARD_NO_SYNC for all SHOW PROC statements.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] kangkaisen commented on a change in pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#discussion_r440572972



##########
File path: fe/src/main/java/org/apache/doris/analysis/ShowProcStmt.java
##########
@@ -82,10 +82,6 @@ public ShowResultSetMetaData getMetaData() {
 
     @Override
     public RedirectStatus getRedirectStatus() {

Review comment:
       I think rename the `forward_to_master` to `show_stmt_forward_to_master`, and change the default value to true is a better way.
   
   **All show stmt should have the consistent behavior.**




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644552857


   > Hi @chaoyli . Actually, not all `ShowProc` stmt should be forward to master.
   > Some of them, yes, like: `show proc "/cluster_balance"`, because the non-master FE has
   > nothing to do the the cluster balance logic.
   > 
   > But some of them don't, like: `show proc "/dbs/10003/10017/partitions/10016/10018";`.
   > This is the stmt to show the tablet info saving in the current FE's memory.
   > 
   > These kind of `show proc` stmt should not be forward to master FE because sometimes
   > we need to check the meta of current FE, to see if there is any inconsistency with the Master FE.
   > 
   > That is why I add the variable `forward_to_master` to control whether to forward these stmts.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644697267


   > > > > > > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   > > > > > 
   > > > > > 
   > > > > > It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.
   > > > > 
   > > > > 
   > > > > So I only make show proc get the forward variable to make a comprise.
   > > > 
   > > > 
   > > > How to do with this?
   > > > `show proc "/dbs/10003/10017/partitions/10016/10018";`
   > > 
   > > 
   > > This is a question. If we make the behavior consistent with master to make balance statistics exists in follower like table metadata.
   > 
   > That could cause too many meta sync log.
   > Maybe we can find a way to classify the `show proc` statement. This can control the behavior of each category.
   
   OK, we should found another method to address the problem.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644680411


   > > > > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   > > > 
   > > > 
   > > > It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.
   > > 
   > > 
   > > So I only make show proc get the forward variable to make a comprise.
   > 
   > How to do with this?
   > `show proc "/dbs/10003/10017/partitions/10016/10018";`
   This is a question. If we make the behavior consistent with master to make balance statistics exists in follower like table metadata.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644647384


   > > > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   > > 
   > > 
   > > It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.
   > 
   > So I only make show proc get the forward variable to make a comprise.
   
   How to do with this?
   `show proc "/dbs/10003/10017/partitions/10016/10018";`


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644557164


   > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   
   It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli removed a comment on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli removed a comment on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644552857


   > Hi @chaoyli . Actually, not all `ShowProc` stmt should be forward to master.
   > Some of them, yes, like: `show proc "/cluster_balance"`, because the non-master FE has
   > nothing to do the the cluster balance logic.
   > 
   > But some of them don't, like: `show proc "/dbs/10003/10017/partitions/10016/10018";`.
   > This is the stmt to show the tablet info saving in the current FE's memory.
   > 
   > These kind of `show proc` stmt should not be forward to master FE because sometimes
   > we need to check the meta of current FE, to see if there is any inconsistency with the Master FE.
   > 
   > That is why I add the variable `forward_to_master` to control whether to forward these stmts.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644606822


   > > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   > 
   > It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.
   
   So I only make show proc get the forward variable to make a comprise.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644694065


   > > > > > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   > > > > 
   > > > > 
   > > > > It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.
   > > > 
   > > > 
   > > > So I only make show proc get the forward variable to make a comprise.
   > > 
   > > 
   > > How to do with this?
   > > `show proc "/dbs/10003/10017/partitions/10016/10018";`
   > 
   > This is a question. If we make the behavior consistent with master to make balance statistics exists in follower like table metadata.
   
   That could cause too many meta sync log.
   Maybe we can find a way to classify the `show proc` statement. This can control the behavior of each category.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644543792


   And by the way, better to fix the compilation problem in another PR.
   That is more urgent and need to be fixed and merged ASAP.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli removed a comment on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli removed a comment on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644680411


   > > > > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   > > > 
   > > > 
   > > > It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.
   > > 
   > > 
   > > So I only make show proc get the forward variable to make a comprise.
   > 
   > How to do with this?
   > `show proc "/dbs/10003/10017/partitions/10016/10018";`
   This is a question. If we make the behavior consistent with master to make balance statistics exists in follower like table metadata.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644554315


   > Hi @chaoyli . Actually, not all `ShowProc` stmt should be forward to master.
   > Some of them, yes, like: `show proc "/cluster_balance"`, because the non-master FE has
   > nothing to do the the cluster balance logic.
   > 
   > But some of them don't, like: `show proc "/dbs/10003/10017/partitions/10016/10018";`.
   > This is the stmt to show the tablet info saving in the current FE's memory.
   > 
   > These kind of `show proc` stmt should not be forward to master FE because sometimes
   > we need to check the meta of current FE, to see if there is any inconsistency with the Master FE.
   > 
   > That is why I add the variable `forward_to_master` to control whether to forward these stmts.
   
   I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And I don't know whether to set the variable or not. The user must remember every show statement which it's semantics. 


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644558471


   > And by the way, better to fix the compilation problem in another PR.
   > That is more urgent and need to be fixed and merged ASAP.
   
   It's because I run the unit test before I push the PR, this is by the way.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644543498


   Hi @chaoyli . Actually, not all `ShowProc` stmt should be forward to master.
   Some of them, yes, like: `show proc "/cluster_balance"`, because the non-master FE has
   nothing to do the the cluster balance logic.
   
   But some of them don't, like: `show proc "/dbs/10003/10017/partitions/10016/10018";`. 
   This is the stmt to show the tablet info saving in the current FE's memory.
   
   These kind of `show proc` stmt should not be forward to master FE because sometimes
   we need to check the meta of current FE, to see if there is any inconsistency with the Master FE.
   
   That is why I add the variable `forward_to_master` to control whether to forward these stmts.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli commented on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644692753


   > > > > I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.
   > > > 
   > > > 
   > > > It indeed complicated. But I can't figure out how to make it simple for user to see both master and non-master FE's meta info.
   > > 
   > > 
   > > So I only make show proc get the forward variable to make a comprise.
   > 
   > How to do with this?
   > `show proc "/dbs/10003/10017/partitions/10016/10018";`
   
   This is a question. If we make the behavior consistent with master to make balance statistics exists in follower like table metadata.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli closed pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli closed pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] chaoyli edited a comment on pull request #3883: [Enhancement] Add FORWARD_NO_SYNC for all SHOW PROC statements

Posted by GitBox <gi...@apache.org>.
chaoyli edited a comment on pull request #3883:
URL: https://github.com/apache/incubator-doris/pull/3883#issuecomment-644554315


   > Hi @chaoyli . Actually, not all `ShowProc` stmt should be forward to master.
   > Some of them, yes, like: `show proc "/cluster_balance"`, because the non-master FE has
   > nothing to do the the cluster balance logic.
   > 
   > But some of them don't, like: `show proc "/dbs/10003/10017/partitions/10016/10018";`.
   > This is the stmt to show the tablet info saving in the current FE's memory.
   > 
   > These kind of `show proc` stmt should not be forward to master FE because sometimes
   > we need to check the meta of current FE, to see if there is any inconsistency with the Master FE.
   > 
   > That is why I add the variable `forward_to_master` to control whether to forward these stmts.
   
   I thinks the variable forward_to_master is useless, even make application complicated. User deploy the Doris will one Master and two Follower. The master will change when new vote. And the user don't know whether to set the variable or not. He must remember every show statement implementation difference.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org