Skip to content

Conversation

@sodonnel
Copy link
Contributor

@sodonnel sodonnel commented Feb 7, 2023

What changes were proposed in this pull request?

In ECBlockInputStream, it is possible to have multiple locations for a single replicaIndex, if, say one replica is decommissioned but still online and there is also an IN_SERVICE replica.

Similar for Maintenance - there can be maintenance and in_service replicas, or just over replicated replicas.

As things stand, if a read to an index fails, it causes a failover to reconstruction read, but there are occassions where it would be possible to read from the spare replica and no need to fall back to reconstruction.

This PR allows spare replicas to be tried before giving up with an error.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-7917

How was this patch tested?

Added a new unit test.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

LGTM


// Now make the spare one error on the next read and we should get an
// error with two failed locations
streamFactory.getBlockStreams().get(3).setThrowException(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain where index 3 is derived from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea I will add a comment for this. Its the order the streams are created in the streamFactory. Indexes 1, 2 and 3 will be read, creating streams 0, 1, and 2. Then when we create another stream to read the new index 1 it will be stream 3.

@sodonnel sodonnel merged commit 11fd5b1 into apache:master Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants