Skip to content

Conversation

@dnhatn
Copy link
Member

@dnhatn dnhatn commented Jul 21, 2018

Today we consider a read request is exhausted if from_seqno is equal to or greater than the max_required_seqno. However, if we stop when from_seqno equals to the max_required_seqno, we will miss an operation whose seqno is max_required_seqno because that operation was not seen yet.

Today we consider a read request is exhausted if from_seqno is equal
to or greater than the max_seqno. However, if we stop when from_seqno
equals to the max_required_seqno, we will miss an operation whose seqno
is max_required_seqno because that operation was not fetched yet.

Our CI https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+ccr+feature-branch-periodic/997/console
found that we missed an operation.
@dnhatn dnhatn added >bug :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features labels Jul 21, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@dnhatn
Copy link
Member Author

dnhatn commented Jul 21, 2018

@dnhatn dnhatn changed the title CCR: Fix incorrect read request complete condition CCR: Fix incorrect read request completion condition Jul 22, 2018
Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Good catch

task.coordinateReads();
ShardChangesAction.Response response = generateShardChangesResponse(0, 64, 1L, 64L);
task.handleReadResponse(0L, 64L, response);
ShardChangesAction.Response response = generateShardChangesResponse(0, 64, 1L, 63L);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if using fromSeq and toSeqNo (instead of size) will result in this being less confusing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@dnhatn
Copy link
Member Author

dnhatn commented Jul 23, 2018

Thanks @bleskes for reviewing.

@dnhatn dnhatn merged commit 8819029 into elastic:ccr Jul 23, 2018
@dnhatn dnhatn deleted the ccr-fix-request-complete-condition branch July 23, 2018 02:14
dnhatn added a commit that referenced this pull request Jul 23, 2018
Today we consider a read request is exhausted if from_seqno is equal to
or greater than the max_required_seqno. However, if we stop when
from_seqno equals to the max_required_seqno, we will miss an operation
whose seqno is max_required_seqno because we have not seen that 
operation yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants