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/04/10 06:32:53 UTC

[GitHub] [incubator-doris] WingsGo opened a new pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

WingsGo opened a new pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293
 
 
   This CL solve problem:
   - FE cann't aware Coordinate BE down and cancel the txns because the
   txns cann't finish.
   - Do some codestyle refactor

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r407060372
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -1389,4 +1389,33 @@ public TransactionState getTransactionStateByCallbackId(long callbackId) {
         }
         return null;
     }
+
+    public List<Long> getTransactionIdByCoordinateBe(String coordinateHost, int limit) {
+        ArrayList<Long> txnIds = new ArrayList<>();
+        readLock();
+        try {
+            idToTransactionState.values().stream()
+                    .filter(t -> (t.getCoordinator().contains(coordinateHost) && (!t.getTransactionStatus().isFinalStatus())))
 
 Review comment:
   In some clusters, FE and BE are deployed together.
   So only by checking `t.getCoordinator().contains(coordinateHost)` will also abort those transactions 
   which the coordinator is FE, not the BE, which is not expected.
   
   The current value of the `coordinator` is an informal value, it is just for displaying the job information. If we want to use this `coordinator` field to do some logic control, we should make it formal, for example, by creating a new class "TxnCoordinator" to save the coordinator info.
   
   Currently, the `coordinator` is either "FE: 192.168.0.1" or "BE: 192.168.0.1". So the new class "TxnCoordinator" may looks like:
   
   ```
   class TxnCoordinator {
       private String source;   // "FE" or "BE"
       private String ip;    // the ip
   }
   ```
   
   And in `getTransactionIdByCoordinateBe`, we should get all transactions with the specified IP and source is BE.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on issue #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on issue #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#issuecomment-614733148
 
 
   > Compile failed.
   > In `DeleteHandler.java`, line 170, there is a `TxnCoordinator` with String Type, you have to change it.
   
   done

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r406647112
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -1389,4 +1389,33 @@ public TransactionState getTransactionStateByCallbackId(long callbackId) {
         }
         return null;
     }
+
+    public List<Long> getTransactionIdByCoordinateBe(String coordinate, int limit) {
 
 Review comment:
   Would better rename `coordinate` to `coordinateHost`

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r407059051
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -1148,8 +1149,7 @@ private void updateDBRunningTxnNum(TransactionStatus preStatus, TransactionState
                 && (curTxnState.getTransactionStatus() == TransactionStatus.PREPARE
                 || curTxnState.getTransactionStatus() == TransactionStatus.COMMITTED)) {
             ++txnNum;
-        } else if (preStatus != null
-                && (preStatus == TransactionStatus.PREPARE
+        } else if ((preStatus == TransactionStatus.PREPARE
 
 Review comment:
   Why removing the `preStatus != null` check?

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r407061906
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -1389,4 +1389,33 @@ public TransactionState getTransactionStateByCallbackId(long callbackId) {
         }
         return null;
     }
+
+    public List<Long> getTransactionIdByCoordinateBe(String coordinateHost, int limit) {
+        ArrayList<Long> txnIds = new ArrayList<>();
+        readLock();
+        try {
+            idToTransactionState.values().stream()
+                    .filter(t -> (t.getCoordinator().contains(coordinateHost) && (!t.getTransactionStatus().isFinalStatus())))
 
 Review comment:
   OK, I will change it.

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r409259272
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/system/HeartbeatMgr.java
 ##########
 @@ -176,6 +176,7 @@ private boolean handleHbResponse(HeartbeatResponse response, boolean isReplay) {
                     if (hbResponse.getStatus() != HbStatus.OK) {
                         // invalid all connections cached in ClientPool
                         ClientPool.backendPool.clearPool(new TNetworkAddress(be.getHost(), be.getBePort()));
+                        Catalog.getCurrentCatalog().getGlobalTransactionMgr().abortTxnWhenCoordinateBeDown(be.getHost(), 100);
 
 Review comment:
   thanks, I see~, and all things is done.

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r409249854
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/system/HeartbeatMgr.java
 ##########
 @@ -176,6 +176,7 @@ private boolean handleHbResponse(HeartbeatResponse response, boolean isReplay) {
                     if (hbResponse.getStatus() != HbStatus.OK) {
                         // invalid all connections cached in ClientPool
                         ClientPool.backendPool.clearPool(new TNetworkAddress(be.getHost(), be.getBePort()));
+                        Catalog.getCurrentCatalog().getGlobalTransactionMgr().abortTxnWhenCoordinateBeDown(be.getHost(), 100);
 
 Review comment:
   I see. 
   Another 2 things:
   1. Plz rebase the code, the FE meta version 82 has been taken.
   2. in `handleHbResponse()` method, if the parameter `isReplay` is true, means it is a meta log replay thread, so we don't need to process the abort in a replay thread(This is not like `clearPool()`, which should also be done in replay thread).

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r408889709
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/system/HeartbeatMgr.java
 ##########
 @@ -176,6 +176,7 @@ private boolean handleHbResponse(HeartbeatResponse response, boolean isReplay) {
                     if (hbResponse.getStatus() != HbStatus.OK) {
                         // invalid all connections cached in ClientPool
                         ClientPool.backendPool.clearPool(new TNetworkAddress(be.getHost(), be.getBePort()));
+                        Catalog.getCurrentCatalog().getGlobalTransactionMgr().abortTxnWhenCoordinateBeDown(be.getHost(), 100);
 
 Review comment:
   because i afraid too many txn cancel will influence the heatbeat function

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r407321003
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -1389,4 +1389,33 @@ public TransactionState getTransactionStateByCallbackId(long callbackId) {
         }
         return null;
     }
+
+    public List<Long> getTransactionIdByCoordinateBe(String coordinateHost, int limit) {
+        ArrayList<Long> txnIds = new ArrayList<>();
+        readLock();
+        try {
+            idToTransactionState.values().stream()
+                    .filter(t -> (t.getCoordinator().contains(coordinateHost) && (!t.getTransactionStatus().isFinalStatus())))
 
 Review comment:
   @morningman Done

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r406655847
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -818,24 +817,26 @@ public boolean isPreviousTransactionsFinished(long endTransactionId, long dbId,
         return true;
     }
 
-    // check if there exists a intersection between the source tableId list and target tableId list
-    // if one of them is null or empty, that means that we don't know related tables in tableList,
-    // we think the two lists may have intersection for right ordered txns
+    /**
+     * check if there exists a intersection between the source tableId list and target tableId list
+     * if one of them is null or empty, that means that we don't know related tables in tableList,
+     * we think the two lists may have intersection for right ordered txns
+     */
     public boolean isIntersectionNotEmpty(List<Long> sourceTableIdList, List<Long> targetTableIdList) {
         if (CollectionUtils.isEmpty(sourceTableIdList) || CollectionUtils.isEmpty(targetTableIdList)) {
             return true;
         }
-        for (int i = 0; i < sourceTableIdList.size(); i++) {
-            for (int j = 0; j < targetTableIdList.size(); j++) {
-                if (sourceTableIdList.get(i).equals(targetTableIdList.get(j))) {
+        for (Long aLong : sourceTableIdList) {
 
 Review comment:
   done

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on issue #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on issue #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#issuecomment-611901121
 
 
   For #3292 

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] morningman merged pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
morningman merged pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r408886582
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/system/HeartbeatMgr.java
 ##########
 @@ -176,6 +176,7 @@ private boolean handleHbResponse(HeartbeatResponse response, boolean isReplay) {
                     if (hbResponse.getStatus() != HbStatus.OK) {
                         // invalid all connections cached in ClientPool
                         ClientPool.backendPool.clearPool(new TNetworkAddress(be.getHost(), be.getBePort()));
+                        Catalog.getCurrentCatalog().getGlobalTransactionMgr().abortTxnWhenCoordinateBeDown(be.getHost(), 100);
 
 Review comment:
   Why here is a limit 100?

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r407061859
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -1148,8 +1149,7 @@ private void updateDBRunningTxnNum(TransactionStatus preStatus, TransactionState
                 && (curTxnState.getTransactionStatus() == TransactionStatus.PREPARE
                 || curTxnState.getTransactionStatus() == TransactionStatus.COMMITTED)) {
             ++txnNum;
-        } else if (preStatus != null
-                && (preStatus == TransactionStatus.PREPARE
+        } else if ((preStatus == TransactionStatus.PREPARE
 
 Review comment:
   because `preStatus == TransactionStatus.PREPARE` contains the `preStatus != null` check

----------------------------------------------------------------
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


With regards,
Apache Git Services

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


[GitHub] [incubator-doris] WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r406655873
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -1389,4 +1389,33 @@ public TransactionState getTransactionStateByCallbackId(long callbackId) {
         }
         return null;
     }
+
+    public List<Long> getTransactionIdByCoordinateBe(String coordinate, int limit) {
 
 Review comment:
   done

----------------------------------------------------------------
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


With regards,
Apache Git Services

---------------------------------------------------------------------
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 #3293: [Transaction]Cancel all txns whose coordinate be is down.

Posted by GitBox <gi...@apache.org>.
kangkaisen commented on a change in pull request #3293: [Transaction]Cancel all txns whose coordinate be is down.
URL: https://github.com/apache/incubator-doris/pull/3293#discussion_r406641632
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java
 ##########
 @@ -818,24 +817,26 @@ public boolean isPreviousTransactionsFinished(long endTransactionId, long dbId,
         return true;
     }
 
-    // check if there exists a intersection between the source tableId list and target tableId list
-    // if one of them is null or empty, that means that we don't know related tables in tableList,
-    // we think the two lists may have intersection for right ordered txns
+    /**
+     * check if there exists a intersection between the source tableId list and target tableId list
+     * if one of them is null or empty, that means that we don't know related tables in tableList,
+     * we think the two lists may have intersection for right ordered txns
+     */
     public boolean isIntersectionNotEmpty(List<Long> sourceTableIdList, List<Long> targetTableIdList) {
         if (CollectionUtils.isEmpty(sourceTableIdList) || CollectionUtils.isEmpty(targetTableIdList)) {
             return true;
         }
-        for (int i = 0; i < sourceTableIdList.size(); i++) {
-            for (int j = 0; j < targetTableIdList.size(); j++) {
-                if (sourceTableIdList.get(i).equals(targetTableIdList.get(j))) {
+        for (Long aLong : sourceTableIdList) {
 
 Review comment:
   Could  rename `aLong` and `value ` to `src` and `target`

----------------------------------------------------------------
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


With regards,
Apache Git Services

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