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

*: support "continue" to label within for-loop #12

Closed
wants to merge 3 commits into from

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Apr 23, 2018

Use case: I've been trying to integrate https://github.com/coreos/etcd/tree/master/tools/local-tester with our functional testing, by setting up a bridge between listen and advertise ports, to simulate network faults (e.g. drop all incoming packets to follower's advertise peer port to simulate an isolated follower).

However, the bridge layer cannot simulate outbound packet drops:

sudo iptables -A INPUT -i ens4 -p tcp -m tcp --dport 2380 -j DROP && sudo iptables -A OUTPUT -o ens4 -p tcp -m tcp --dport 2380 -j DROP

It can only simulate append/snapshot message drops but not leader heartbeat drops (rafthttp creates an HTTP client to the leader's advertise peer port to poll leader heartbeats).

To summarize, we need continue support, to simulate leader heartbeat dropping (trying to extend it to test etcd-io/etcd#9333 and etcd-io/etcd#9563):

This PR adds a support for "continue" + label, as below:

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

continue failpoint must have matching gofail comments: one to define continue label, the other to define non-blocking Eval method call (since defer would block DELETE calls).

gofail enable ./rafthttp would output:

diff --git a/rafthttp/stream.go b/rafthttp/stream.go
--- a/rafthttp/stream.go
+++ b/rafthttp/stream.go
@@ -510,7 +510,7 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error {
        }
        cr.mu.Unlock()
 
-       // gofail: var raftDropHeartbeat continue
+__fp_raftDropHeartbeat:
        for {
                m, err := dec.decode()
                if err != nil {
@@ -520,7 +520,7 @@ func (cr *streamReader) decodeLoop(rc io.ReadCloser, t streamType) error {
                        return err
                }
 
-               // gofail: var raftDropHeartbeat bool eval
+               if vraftDropHeartbeat, __fpErr := __fp_raftDropHeartbeat.Eval(); __fpErr == nil { _, __fpTypeOK := vraftDropHeartbeat.(bool); if !__fpTypeOK { goto __badTyperaftDropHeartbeat } else { continue __fp_raftDropHeartbeat }; __badTyperaftDropHeartbeat: __fp_raftDropHeartbeat.BadType(vraftDropHeartbeat, "bool"); };
                receivedBytes.WithLabelValues(types.ID(m.From).String()).Add(float64(m.Size()))

I tried it, and it seems to work well for our use case (gofail disable ./rafthttp works as well).

Any thoughts or better way? @heyitsanthony

Thanks!

var skip continue // for label
var skip bool // to trigger "continue" on above label

Signed-off-by: Gyuho Lee <[email protected]>
@gyuho
Copy link
Contributor Author

gyuho commented Apr 23, 2018

Btw, the workflow would be:

curl -L http://localhost:22381

# start heartbeat drop in follower
curl \
  -L http://localhost:22381/github.com/coreos/etcd/rafthttp/raftDropHeartbeat \
  -X PUT -d'return(true)'

# remove failpoints
curl \
  -L http://localhost:22381/github.com/coreos/etcd/rafthttp/raftDropHeartbeat \
  -X DELETE

@heyitsanthony
Copy link
Contributor

@gyuho my thinking is this overloads too much of the syntax; var abc continue isn't defining a variable. I'll see if I can come up with something cleaner tonight.

@gyuho
Copy link
Contributor Author

gyuho commented Apr 23, 2018

@heyitsanthony That would be awesome (nothing urgent in etcd side)! Thanks!

@gyuho
Copy link
Contributor Author

gyuho commented Apr 24, 2018

Closing in favor of #13.

Thanks!

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.

2 participants