Skip to content

Commit aab0f4a

Browse files
committed
optimize by review
1 parent 72644fe commit aab0f4a

File tree

6 files changed

+31
-31
lines changed

6 files changed

+31
-31
lines changed

src/main/java/com/alipay/oceanbase/rpc/ObTableClient.java

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import com.alipay.oceanbase.rpc.threadlocal.ThreadLocalMap;
4747
import com.alipay.oceanbase.rpc.util.*;
4848
import com.alipay.remoting.util.StringUtils;
49-
import com.google.common.annotations.VisibleForTesting;
5049
import org.slf4j.Logger;
5150

5251
import java.util.*;
@@ -494,7 +493,7 @@ private <T> T execute(String tableName, TableExecuteCallback<T> callback, ObServ
494493
// about routing problems, ODP will retry on their side
495494
if (ex instanceof ObTableException) {
496495
// errors needed to retry will retry until timeout
497-
if (((ObTableException) ex).isNeedRetryServerError()) {
496+
if (((ObTableException) ex).isNeedRetryError()) {
498497
logger.warn(
499498
"execute while meet server error in odp mode, need to retry, errorCode: {} , errorMsg: {}, try times {}",
500499
((ObTableException) ex).getErrorCode(), ex.getMessage(),
@@ -516,13 +515,12 @@ private <T> T execute(String tableName, TableExecuteCallback<T> callback, ObServ
516515
logger.warn("retry when replica not readable: {}", ex.getMessage());
517516
route.addToBlackList(tableParam.getObTable().getIp());
518517
} else {
519-
logger.warn("retry to timeout when replica not readable: {}",
520-
ex.getMessage());
518+
logger.warn("timeout, cause replica is not readable, tryTimes={}", tryTimes);
521519
RUNTIME.error("replica not readable", ex);
522520
throw ex;
523521
}
524522
} else if (ex instanceof ObTableException
525-
&& (((ObTableException) ex).isNeedRefreshTableEntry() || ((ObTableException) ex).isNeedRetryServerError())) {
523+
&& (((ObTableException) ex).isNeedRefreshTableEntry() || ((ObTableException) ex).isNeedRetryError())) {
526524
if (ex instanceof ObTableNotExistException) {
527525
String logMessage = String.format(
528526
"exhaust retry while meet TableNotExist Exception, table name: %s, errorCode: %d",
@@ -537,7 +535,7 @@ private <T> T execute(String tableName, TableExecuteCallback<T> callback, ObServ
537535
tableRoute.refreshMeta(tableName);
538536
// reset failure count while fetch all route info
539537
this.resetExecuteContinuousFailureCount(tableName);
540-
} else if (((ObTableException) ex).isNeedRetryServerError()) {
538+
} else if (((ObTableException) ex).isNeedRetryError()) {
541539
// retry server errors, no need to refresh partition location
542540
needRefreshPartitionLocation = false;
543541
logger.warn(
@@ -712,7 +710,7 @@ private <T> T execute(String tableName, OperationExecuteCallback<T> callback,
712710
// about routing problems, ODP will retry on their side
713711
if (ex instanceof ObTableException) {
714712
// errors needed to retry will retry until timeout
715-
if (((ObTableException) ex).isNeedRetryServerError()) {
713+
if (((ObTableException) ex).isNeedRetryError()) {
716714
logger.warn(
717715
"execute while meet server error in odp mode, need to retry, errorCode: {} , errorMsg: {}, try times {}",
718716
((ObTableException) ex).getErrorCode(), ex.getMessage(),
@@ -734,12 +732,12 @@ private <T> T execute(String tableName, OperationExecuteCallback<T> callback,
734732
logger.warn("retry when replica not readable: {}", ex.getMessage());
735733
route.addToBlackList(tableParam.getObTable().getIp());
736734
} else {
737-
logger.warn("retry to timeout when replica not readable: {}", ex.getMessage());
735+
logger.warn("timeout, cause replica is not readable, tryTimes={}", tryTimes);
738736
RUNTIME.error("replica not readable", ex);
739737
throw ex;
740738
}
741739
} else if (ex instanceof ObTableException
742-
&& (((ObTableException) ex).isNeedRefreshTableEntry() || ((ObTableException) ex).isNeedRetryServerError())) {
740+
&& (((ObTableException) ex).isNeedRefreshTableEntry() || ((ObTableException) ex).isNeedRetryError())) {
743741
if (ex instanceof ObTableNotExistException) {
744742
String logMessage = String.format(
745743
"exhaust retry while meet TableNotExist Exception, table name: %s, errorCode: %d",
@@ -757,7 +755,7 @@ private <T> T execute(String tableName, OperationExecuteCallback<T> callback,
757755
tableRoute.refreshMeta(tableName);
758756
// reset failure count while fetch all route info
759757
this.resetExecuteContinuousFailureCount(tableName);
760-
} else if (((ObTableException) ex).isNeedRetryServerError()) {
758+
} else if (((ObTableException) ex).isNeedRetryError()) {
761759
// retry server errors, no need to refresh partition location
762760
needRefreshPartitionLocation = false;
763761
logger.warn(
@@ -841,23 +839,23 @@ public void resetExecuteContinuousFailureCount(String tableName) {
841839

842840
/**
843841
* refresh all ob server synchronized just in case rslist has changed, it will not refresh if last refresh time is 1 min ago
844-
* @param forceRenew flag to force refresh the rsList if changes happen
842+
* @param forceRefresh flag to force refresh the rsList if changes happen
845843
* 1. cannot find table from tables, need refresh tables
846844
* 2. server list refresh failed: {see com.alipay.oceanbase.obproxy.resource.ObServerStateProcessor#MAX_REFRESH_FAILURE}
847845
*
848846
* @throws Exception if fail
849847
*/
850-
public void syncRefreshMetadata(boolean forceRenew) throws Exception {// do not refresh within 5 seconds even if forceRenew
848+
public void syncRefreshMetadata(boolean forceRefresh) throws Exception {// do not refresh within 5 seconds even if forceRenew
851849
checkStatus();
852850
long lastRefreshMetadataTimestamp = tableRoute.getLastRefreshMetadataTimestamp();
853851
if (System.currentTimeMillis() - lastRefreshMetadataTimestamp < tableEntryRefreshLockTimeout) {
854852
logger
855853
.warn(
856-
"have to wait for more than 5 seconds to refresh metadata, it has refreshed at: {}",
857-
lastRefreshMetadataTimestamp);
854+
"have to wait for more than {} seconds to refresh metadata, it has refreshed at: {}",
855+
tableEntryRefreshLockTimeout, lastRefreshMetadataTimestamp);
858856
return;
859857
}
860-
if (!forceRenew
858+
if (!forceRefresh
861859
&& System.currentTimeMillis() - lastRefreshMetadataTimestamp < metadataRefreshInterval) {
862860
logger
863861
.warn(
@@ -878,14 +876,15 @@ public void syncRefreshMetadata(boolean forceRenew) throws Exception {// do not
878876
}
879877
try {
880878
// double check timestamp
879+
lastRefreshMetadataTimestamp = tableRoute.getLastRefreshMetadataTimestamp();
881880
if (System.currentTimeMillis() - lastRefreshMetadataTimestamp < tableEntryRefreshLockTimeout) {
882881
logger
883882
.warn(
884-
"have to wait for more than 5 seconds to refresh metadata, it has refreshed at: {}",
885-
lastRefreshMetadataTimestamp);
883+
"have to wait for more than {} seconds to refresh metadata, it has refreshed at: {}",
884+
tableEntryRefreshLockTimeout, lastRefreshMetadataTimestamp);
886885
return;
887886
}
888-
if (!forceRenew
887+
if (!forceRefresh
889888
&& System.currentTimeMillis() - lastRefreshMetadataTimestamp < metadataRefreshInterval) {
890889
logger
891890
.warn(
@@ -1041,7 +1040,7 @@ public List<ObTableParam> getTableParams(String tableName, List<Row> rowkeys) th
10411040
}
10421041

10431042
/**
1044-
* 根据 start-end 获取 partition ids addrs
1043+
* get partition ids and addrs by start-end range
10451044
* @param tableName table want to get
10461045
* @param query query
10471046
* @param start start key
@@ -2292,7 +2291,7 @@ public ObPayload execute(final ObTableAbstractOperationRequest request) throws E
22922291
} catch (Exception ex) {
22932292
tryTimes++;
22942293
if (ex instanceof ObTableException &&
2295-
(((ObTableException) ex).isNeedRefreshTableEntry() || ((ObTableException) ex).isNeedRetryServerError())) {
2294+
(((ObTableException) ex).isNeedRefreshTableEntry() || ((ObTableException) ex).isNeedRetryError())) {
22962295
logger.warn(
22972296
"tablename:{} partition id:{} batch ops refresh table while meet ObTableMasterChangeException, errorCode: {}",
22982297
request.getTableName(), request.getPartitionId(), ((ObTableException) ex).getErrorCode(), ex);
@@ -2808,8 +2807,11 @@ public String getCurrentIDC() {
28082807
return this.currentIDC;
28092808
}
28102809

2811-
@VisibleForTesting
2810+
/**
2811+
* only use in test whether the background thread works normally to expand connection pool
2812+
* */
28122813
public int getConnectionNum() {
2814+
checkStatus();
28132815
ObTable randomTable = tableRoute.getFirstObTable();
28142816
return randomTable.getConnectionNum();
28152817
}

src/main/java/com/alipay/oceanbase/rpc/exception/ObTableException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public boolean isNeedRefreshTableEntry() {
8484
return needRefreshTableEntry;
8585
}
8686

87-
public boolean isNeedRetryServerError() {
87+
public boolean isNeedRetryError() {
8888
return errorCode == ResultCodes.OB_TRY_LOCK_ROW_CONFLICT.errorCode
8989
|| errorCode == ResultCodes.OB_TRANSACTION_SET_VIOLATION.errorCode
9090
|| errorCode == ResultCodes.OB_SCHEMA_EAGAIN.errorCode;

src/main/java/com/alipay/oceanbase/rpc/location/model/TableGroupCache.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public String tryGetTableNameFromTableGroupCache(final String tableGroupName,
103103
}
104104

105105
/**
106-
* 根据 tableGroup 获取其中一个tableName
106+
* get the first table's name in the tableGroup
107107
* physicalTableName Complete table from table group
108108
* @param physicalTableName
109109
* @param tableGroupName

src/main/java/com/alipay/oceanbase/rpc/protocol/payload/impl/execute/query/AbstractQueryStreamResult.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ protected ObPayload commonExecute(ObTableClient client, Logger logger,
182182
if (client.isOdpMode()) {
183183
// if exceptions need to retry, retry to timeout
184184
if (e instanceof ObTableException
185-
&& ((ObTableException) e).isNeedRetryServerError()) {
185+
&& ((ObTableException) e).isNeedRetryError()) {
186186
logger
187187
.warn(
188188
"tablename:{} stream query execute while meet Exception in odp mode needing retry, errorCode: {}, errorMsg: {}, try times {}",
@@ -268,7 +268,7 @@ protected ObPayload commonExecute(ObTableClient client, Logger logger,
268268
client.calculateContinuousFailure(indexTableName, e.getMessage());
269269
throw new ObTableRetryExhaustedException(logMessage, e);
270270
}
271-
} else if (((ObTableException) e).isNeedRetryServerError()) {
271+
} else if (((ObTableException) e).isNeedRetryError()) {
272272
// retry server errors, no need to refresh partition location
273273
needRefreshPartitionLocation = false;
274274
if (client.isRetryOnChangeMasterTimes()) {

src/main/java/com/alipay/oceanbase/rpc/table/ObTableClientBatchOpsImpl.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
package com.alipay.oceanbase.rpc.table;
1919

20-
import com.alipay.oceanbase.rpc.ObGlobal;
2120
import com.alipay.oceanbase.rpc.ObTableClient;
2221
import com.alipay.oceanbase.rpc.bolt.transport.TransportCodes;
2322
import com.alipay.oceanbase.rpc.exception.*;
@@ -392,7 +391,7 @@ public void partitionExecute(ObTableOperationResult[] results,
392391
if (obTableClient.isOdpMode()) {
393392
// if exceptions need to retry, retry to timeout
394393
if (ex instanceof ObTableException
395-
&& ((ObTableException) ex).isNeedRetryServerError()) {
394+
&& ((ObTableException) ex).isNeedRetryError()) {
396395
logger.warn(
397396
"meet need retry exception when execute normal batch in odp mode."
398397
+ "tableName: {}, errMsg: {}", tableName, ex.getMessage());
@@ -433,7 +432,7 @@ public void partitionExecute(ObTableOperationResult[] results,
433432
obTableClient.calculateContinuousFailure(tableName, ex.getMessage());
434433
throw new ObTableRetryExhaustedException(logMessage, ex);
435434
}
436-
} else if (((ObTableException) ex).isNeedRetryServerError()) {
435+
} else if (((ObTableException) ex).isNeedRetryError()) {
437436
// retry server errors, no need to refresh partition location
438437
needRefreshPartitionLocation = false;
439438
if (obTableClient.isRetryOnChangeMasterTimes()) {

src/main/java/com/alipay/oceanbase/rpc/table/ObTableClientLSBatchOpsImpl.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
package com.alipay.oceanbase.rpc.table;
1919

20-
import com.alipay.oceanbase.rpc.ObGlobal;
2120
import com.alipay.oceanbase.rpc.ObTableClient;
2221
import com.alipay.oceanbase.rpc.bolt.transport.TransportCodes;
2322
import com.alipay.oceanbase.rpc.checkandmutate.CheckAndInsUp;
@@ -654,7 +653,7 @@ public void partitionExecute(ObTableSingleOpResult[] results,
654653
needRefreshPartitionLocation = true;
655654
if (obTableClient.isOdpMode()) {
656655
// if exceptions need to retry, retry to timeout
657-
if (ex instanceof ObTableException && ((ObTableException) ex).isNeedRetryServerError()) {
656+
if (ex instanceof ObTableException && ((ObTableException) ex).isNeedRetryError()) {
658657
logger.warn("meet need retry exception when execute ls batch in odp mode." +
659658
"tableName: {}, errMsg: {}", realTableName, ex.getMessage());
660659
} else {
@@ -707,7 +706,7 @@ public void partitionExecute(ObTableSingleOpResult[] results,
707706
obTableClient.calculateContinuousFailure(realTableName, ex.getMessage());
708707
throw new ObTableRetryExhaustedException(logMessage, ex);
709708
}
710-
} else if (((ObTableException) ex).isNeedRetryServerError()) {
709+
} else if (((ObTableException) ex).isNeedRetryError()) {
711710
// retry server errors, no need to refresh partition location
712711
needRefreshPartitionLocation = false;
713712
if (obTableClient.isRetryOnChangeMasterTimes()) {

0 commit comments

Comments
 (0)