Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: add "gofail-go" for manual "Release" call #15

Merged
merged 3 commits into from
Aug 1, 2019

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jun 13, 2018

To address #14.

ref. #12 and #13.

/cc @heyitsanthony

To reiterate etcd use case, we want to simulate leader heartbeat drop in incoming traffic:

iff --git a/etcdserver/api/rafthttp/stream.go b/etcdserver/api/rafthttp/stream.go
index f6cda71a3..b8795bff0 100644
--- a/etcdserver/api/rafthttp/stream.go
+++ b/etcdserver/api/rafthttp/stream.go
@@ -510,6 +510,7 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error {
        }
        cr.mu.Unlock()
 
+       // gofail: labelRaftDropHeartbeat:
        for {
                m, err := dec.decode()
                if err != nil {
@@ -519,6 +520,8 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error {
                        return err
                }
 
+               // gofail-go: var raftDropHeartbeat struct{}
+               // continue labelRaftDropHeartbeat
                receivedBytes.WithLabelValues(types.ID(m.From).String()).Add(float64(m.Size()))
 
                cr.mu.Lock()

With this patch, gofail enable would output:

diff --git a/etcdserver/api/rafthttp/stream.go b/etcdserver/api/rafthttp/stream.go
index b8795bff0..543ada74f 100644
--- a/etcdserver/api/rafthttp/stream.go
+++ b/etcdserver/api/rafthttp/stream.go
@@ -510,7 +510,7 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error {
        }
        cr.mu.Unlock()
 
-       // gofail: labelRaftDropHeartbeat:
+       /* gofail-label */ labelRaftDropHeartbeat:
        for {
                m, err := dec.decode()
                if err != nil {
@@ -520,8 +520,8 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error {
                        return err
                }
 
-               // gofail-go: var raftDropHeartbeat struct{}
-               // continue labelRaftDropHeartbeat
+               if vraftDropHeartbeat, __fpGoErr := __fp_raftDropHeartbeat.Acquire(); __fpGoErr == nil { go __fp_raftDropHeartbeat.Release(); _, __fpTypeOK := vraftDropHeartbeat.(struct{}); if !__fpTypeOK { goto __badTyperaftDropHeartbeat} 
+                        continue labelRaftDropHeartbeat; __badTyperaftDropHeartbeat: __fp_raftDropHeartbeat.BadType(vraftDropHeartbeat, "struct{}"); };
                receivedBytes.WithLabelValues(types.ID(m.From).String()).Add(float64(m.Size()))
 
                cr.mu.Lock()

Now, *Failpoint.Release will be waiting to be triggered in a goroutine, instead of never being called with defer (triggered when gofail disables the corresponding Failpoint). This adds released bool field to safely release multiple goroutines.

#14 (comment) originally suggested

// gofail-go: to indicate that a delete will prevent new fp's from being issued, but has no control over whether any are inflight.

This PR's // gofail-go: does control queued Release calls. Once disabled, it will prevent new failpoints from being issued. This PR tries to enable manual Release calls via disable request, because without control over inflight Release calls, the write lock for disable request would block, thus unable to delete the failpoints in the first place.

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant