Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

Replicator: Do not accept SetAckOffset before the cg extent is created locally #251

Merged
merged 3 commits into from
Jul 19, 2017

Conversation

datoug
Copy link
Contributor

@datoug datoug commented Jul 17, 2017

No description provided.

@datoug datoug requested a review from thuningxu July 17, 2017 20:57
@coveralls
Copy link

coveralls commented Jul 17, 2017

Coverage Status

Changes Unknown when pulling b94ec3a on r_race into ** on master**.

@datoug datoug requested a review from kirg July 18, 2017 19:02
@@ -67,6 +67,9 @@ type (
storehostConn map[string]*outConnection
storehostConnMutex sync.RWMutex

knownCgExtents map[string]struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

May I suggest using the "common/set" library .. it's really just syntactic sugar, but I think it makes code look better.

r.m3Client.IncCounter(metrics.ReplicatorSetAckOffsetScope, metrics.ReplicatorFailures)
return err
}
if len(extent.GetExtent().GetStoreUUIDs()) < 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

"ReadConsumerGroupExtent" (in the metadata client), already checks for this condition .. and returns an error. So the check here is redundant .. and will likely never be reached.

@coveralls
Copy link

coveralls commented Jul 19, 2017

Coverage Status

Coverage increased (+0.07%) to 68.437% when pulling 81a507a on r_race into 9f74915 on master.

@datoug datoug merged commit 98771ed into master Jul 19, 2017
@datoug datoug deleted the r_race branch July 19, 2017 03:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants