does not auto resetting when we manual control

[lrita]

Does not auto resetting when we manual control by Break().

---

This change is 

[lrita]

Hi @ajwerner,
Could you have time to review this patch?

[ajwerner]

Thanks for the patch, this does seem like a bug given the comments. Can you write a unit test?

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lrita)

---

circuitbreaker.go, line 240 at r1 (raw file):

// return true.
func (cb *Breaker) Trip() {
	// should happen before Tripped()

This comment doesn't clarify much for me. I think you mean the reading of the time should happen before setting the atomics. Is that really causing any problems for you?

I don't mind the re-ordering of the statements but the comment is if anything more confusing than helpful because it doesn't say what should happen before Tripped().

[lrita]

@ajwerner there is a concurrent issue between setting tripped and reading lastFailure. It make state() to read an invalid lastFailure when there occurs a latency between setting tripped and lastFailure in Trip().

Is it really cause a bug? Yes to original version of circuitbreaker, but no for current version. Because of 79eae79 make the exported Trip() meaningless.

We can think that there are many goroutines invoking Call concurrently, and they will invoke Success() if the Call's callback return nil. If we want make the breaker open a moment and let it auto recovery, we can invoke the Trip() manually. And the concurrent issue make the circuitbreaker recovery immediately by reading invalid lastFailure and becoming into halfopen at wrong time.

After 79eae79 we redefinition the mechanism of circuitbreaker and exported Trip() is meaningless, hence the bug scenario is disappeared.

I can delete the comment if you feel confused.

[ajwerner]

I think I'm willing to merge this, mostly because we don't use Break anywhere outside of testing but this change really could benefit from a unit test so I'll make one more appeal.