You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2020/10/20 02:54:27 UTC
[GitHub] [iotdb] neuyilan opened a new pull request #1836: [IOTDB-948] Use binary search when checking the follower's match index when catchup
neuyilan opened a new pull request #1836:
URL: https://github.com/apache/iotdb/pull/1836
Now, when the follower lags behind the leader a lot, one catchup task will be triggered. When catchup, it will judge whether a certain log saved in the leader's memory is consistent with the follower. If so, the leader can use logs in its memory to catch up. If the leader cannot find a log in memory that matches the follower, the leader needs to send snapshots.
When checking whether the leader's log match with the follower's, the current implementation starts from the largest index of the stored log in memory, and traverses from large to small to see whether it is consistent with the follower. However, each traversal will send one RPC. Since the maximum number of logs in memory is 1000, there may be 1000 times of RPC, and the time is O( n ). If it becomes a binary search, it only needs O(log n), and 1000 logs only need 10 RPC communications, it'll save communication cost.
----------------------------------------------------------------
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
[GitHub] [iotdb] jt2594838 commented on a change in pull request #1836: [IOTDB-948] Use binary search when checking the follower's match index when catchup
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #1836:
URL: https://github.com/apache/iotdb/pull/1836#discussion_r512363188
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -1691,7 +1693,7 @@ protected TSStatus executeNonQueryPlan(PhysicalPlan plan) {
} catch (QueryProcessException e) {
logger.error("meet error while processing non-query. ", e);
Throwable cause = e;
- while (cause.getCause() != null){
+ while (cause.getCause() != null && cause.getCause().getCause() != null) {
Review comment:
I think it would be better to think from the user side. Such exceptions does few meaning to users, so we'd better remove them for clarity.
----------------------------------------------------------------
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
[GitHub] [iotdb] neuyilan commented on a change in pull request #1836: [IOTDB-948] Use binary search when checking the follower's match index when catchup
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #1836:
URL: https://github.com/apache/iotdb/pull/1836#discussion_r510570408
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -1691,7 +1693,7 @@ protected TSStatus executeNonQueryPlan(PhysicalPlan plan) {
} catch (QueryProcessException e) {
logger.error("meet error while processing non-query. ", e);
Throwable cause = e;
- while (cause.getCause() != null){
+ while (cause.getCause() != null && cause.getCause().getCause() != null) {
Review comment:
this is used to fix the UT tests that the exception only shows the message bug but lacks the exception type. I saw you have fixed in the commit[1], but I think it's better to be consistent with the single server.
So if you agree with me, I would re-add the exception type you've deleted in [1], otherwise, I'll change the code here as normal.
[1] https://github.com/apache/iotdb/commit/83e916a9e7bc618f348b7078bd79e62db6d9d546
----------------------------------------------------------------
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
[GitHub] [iotdb] neuyilan commented on a change in pull request #1836: [IOTDB-948] Use binary search when checking the follower's match index when catchup
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #1836:
URL: https://github.com/apache/iotdb/pull/1836#discussion_r510640073
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -1691,7 +1693,7 @@ protected TSStatus executeNonQueryPlan(PhysicalPlan plan) {
} catch (QueryProcessException e) {
logger.error("meet error while processing non-query. ", e);
Throwable cause = e;
- while (cause.getCause() != null){
+ while (cause.getCause() != null && cause.getCause().getCause() != null) {
Review comment:
have changed the code as normal
----------------------------------------------------------------
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
[GitHub] [iotdb] neuyilan commented on a change in pull request #1836: [IOTDB-948] Use binary search when checking the follower's match index when catchup
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #1836:
URL: https://github.com/apache/iotdb/pull/1836#discussion_r510570408
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -1691,7 +1693,7 @@ protected TSStatus executeNonQueryPlan(PhysicalPlan plan) {
} catch (QueryProcessException e) {
logger.error("meet error while processing non-query. ", e);
Throwable cause = e;
- while (cause.getCause() != null){
+ while (cause.getCause() != null && cause.getCause().getCause() != null) {
Review comment:
this is used to fix the IT tests that the exception only shows the message bug but lacks the exception type. I saw you have fixed in the commit[1], but I think it's better to be consistent with the single server.
So if you agree with me, I would re-add the exception type you've deleted in [1], otherwise, I'll change the code here as normal.
[1] https://github.com/apache/iotdb/commit/83e916a9e7bc618f348b7078bd79e62db6d9d546
----------------------------------------------------------------
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
[GitHub] [iotdb] jt2594838 merged pull request #1836: [IOTDB-948] Use binary search when checking the follower's match index when catchup
Posted by GitBox <gi...@apache.org>.
jt2594838 merged pull request #1836:
URL: https://github.com/apache/iotdb/pull/1836
----------------------------------------------------------------
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
[GitHub] [iotdb] jt2594838 commented on a change in pull request #1836: [IOTDB-948] Use binary search when checking the follower's match index when catchup
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #1836:
URL: https://github.com/apache/iotdb/pull/1836#discussion_r510555104
##########
File path: server/src/main/java/org/apache/iotdb/db/service/TSServiceImpl.java
##########
@@ -1691,7 +1693,7 @@ protected TSStatus executeNonQueryPlan(PhysicalPlan plan) {
} catch (QueryProcessException e) {
logger.error("meet error while processing non-query. ", e);
Throwable cause = e;
- while (cause.getCause() != null){
+ while (cause.getCause() != null && cause.getCause().getCause() != null) {
Review comment:
Could you please explain this?
----------------------------------------------------------------
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