etcd: Compare not equal operator is broken in etcd

I couldn’t believe it myself, and after a long day of hacking, I finally assumed it was something in etcd’s code that was broken instead of mine. Here’s a simple reproducer and logs.

What are we using?

$ etcd --version
etcd Version: 3.3.12
Git SHA: d57e8b8d9
Go Version: go1.10.8
Go OS/Arch: linux/amd64

Run etcd in an empty dir:

etcd
2019-03-20 14:49:36.168882 I | etcdmain: etcd Version: 3.3.12
2019-03-20 14:49:36.168944 I | etcdmain: Git SHA: d57e8b8d9
2019-03-20 14:49:36.168955 I | etcdmain: Go Version: go1.10.8
2019-03-20 14:49:36.168963 I | etcdmain: Go OS/Arch: linux/amd64
2019-03-20 14:49:36.168971 I | etcdmain: setting maximum number of CPUs to 4, total number of available CPUs is 4
2019-03-20 14:49:36.168982 W | etcdmain: no data-dir provided, using default data-dir ./default.etcd
2019-03-20 14:49:36.169433 I | embed: listening for peers on http://localhost:2380
2019-03-20 14:49:36.169547 I | embed: listening for client requests on localhost:2379
2019-03-20 14:49:36.169939 I | etcdserver: name = default
2019-03-20 14:49:36.169953 I | etcdserver: data dir = default.etcd
2019-03-20 14:49:36.169963 I | etcdserver: member dir = default.etcd/member
2019-03-20 14:49:36.169968 I | etcdserver: heartbeat = 100ms
2019-03-20 14:49:36.169972 I | etcdserver: election = 1000ms
2019-03-20 14:49:36.169979 I | etcdserver: snapshot count = 100000
2019-03-20 14:49:36.169993 I | etcdserver: advertise client URLs = http://localhost:2379
2019-03-20 14:49:36.170001 I | etcdserver: initial advertise peer URLs = http://localhost:2380
2019-03-20 14:49:36.170014 I | etcdserver: initial cluster = default=http://localhost:2380
2019-03-20 14:49:36.179266 I | etcdserver: starting member 8e9e05c52164694d in cluster cdf818194e3a8c32
2019-03-20 14:49:36.179303 I | raft: 8e9e05c52164694d became follower at term 0
2019-03-20 14:49:36.179319 I | raft: newRaft 8e9e05c52164694d [peers: [], term: 0, commit: 0, applied: 0, lastindex: 0, lastterm: 0]
2019-03-20 14:49:36.179328 I | raft: 8e9e05c52164694d became follower at term 1
2019-03-20 14:49:36.180643 W | auth: simple token is not cryptographically signed
2019-03-20 14:49:36.181095 I | etcdserver: starting server... [version: 3.3.12, cluster version: to_be_decided]
2019-03-20 14:49:36.181467 I | etcdserver: 8e9e05c52164694d as single-node; fast-forwarding 9 ticks (election ticks 10)
2019-03-20 14:49:36.182013 I | etcdserver/membership: added member 8e9e05c52164694d [http://localhost:2380] to cluster cdf818194e3a8c32
2019-03-20 14:49:37.179884 I | raft: 8e9e05c52164694d is starting a new election at term 1
2019-03-20 14:49:37.179965 I | raft: 8e9e05c52164694d became candidate at term 2
2019-03-20 14:49:37.180035 I | raft: 8e9e05c52164694d received MsgVoteResp from 8e9e05c52164694d at term 2
2019-03-20 14:49:37.180098 I | raft: 8e9e05c52164694d became leader at term 2
2019-03-20 14:49:37.180139 I | raft: raft.node: 8e9e05c52164694d elected leader 8e9e05c52164694d at term 2
2019-03-20 14:49:37.180583 I | etcdserver: setting up the initial cluster version to 3.3
2019-03-20 14:49:37.180998 N | etcdserver/membership: set the initial cluster version to 3.3
2019-03-20 14:49:37.181059 I | embed: ready to serve client requests
2019-03-20 14:49:37.181239 I | etcdserver/api: enabled capabilities for version 3.3
2019-03-20 14:49:37.181303 I | etcdserver: published {Name:default ClientURLs:[http://localhost:2379]} to cluster cdf818194e3a8c32
2019-03-20 14:49:37.181539 E | etcdmain: forgot to set Type=notify in systemd service file?
2019-03-20 14:49:37.182692 N | embed: serving insecure client requests on 127.0.0.1:2379, this is strongly discouraged!

Here’s a golang example to trigger the issue:

package main

import (
	"context"
	"log"

	etcd "github.com/coreos/etcd/clientv3"
)

func main() {
	log.Printf("hello")
	cfg := etcd.Config{
		Endpoints: []string{"http://127.0.0.1:2379"},
		// 0 disables auto-sync. By default auto-sync is disabled.
		AutoSyncInterval: 0,
	}
	client, err := etcd.New(cfg) // connect!
	if err != nil {
		log.Fatal(err)
	}
	log.Printf("connected")

	ifs := []etcd.Cmp{}
	ops := []etcd.Op{}
	//els := []etcd.Op{}

	key := "/_hello/foo"
	data := "world"

	// works
	//ifs = append(ifs, etcd.Compare(etcd.Value(key), "=", data)) // desired state
	//els = append(ops, etcd.OpPut(key, data)) // TODO: add a TTL? (etcd.WithLease)

	ifs = append(ifs, etcd.Compare(etcd.Value(key), "!=", data)) // desired state
	ops = append(ops, etcd.OpPut(key, data))                     // TODO: add a TTL? (etcd.WithLease)

	log.Printf("txn1...")
	//resp1, err := client.KV.Txn(context.TODO()).If(ifs...).Then([]etcd.Op{}...).Else(els...).Commit()
	resp1, err := client.KV.Txn(context.TODO()).If(ifs...).Then(ops...).Else([]etcd.Op{}...).Commit()
	log.Printf("result1: %+v || %+v", resp1, err) // succeeded is set to true if the compare evaluated to true

	log.Printf("txn2...")
	//resp2, err := client.KV.Txn(context.TODO()).If(ifs...).Then([]etcd.Op{}...).Else(els...).Commit()
	resp2, err := client.KV.Txn(context.TODO()).If(ifs...).Then(ops...).Else([]etcd.Op{}...).Commit()
	log.Printf("result2: %+v || %+v", resp2, err) // succeeded is set to true if the compare evaluated to true

	// test with: ETCDCTL_API=3 etcdctl get / --prefix
}

When the golang code is run, the following appears in the etcd server logs:

proto: no coders for int
proto: no encoder for ValueSize int [GetProperties]
proto: no coders for int
proto: no encoder for ValueSize int [GetProperties]

And running etcdctl produces no output:

$ ETCDCTL_API=3 etcdctl get / --prefix

If you modify the above golang example to use = instead of !=, and switch the then for the else case, then it works correctly as expected.

Thoughts welcome!

Thanks

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 2
  • Comments: 39 (15 by maintainers)

Most upvoted comments

I do not think the current behavior is logically correct, because both of the following comparisons will be evaluated as false. But I am not sure what the “correct” behavior should be. Regarding the idea of returning error when this happens, logically this is probably the right thing to do, but I think it is a pretty hard break on user-facing API.

  • clientv3.Compare(clientv3.Value(non-exist-key), "=", any-value)
  • clientv3.Compare(clientv3.Value(non-exist-key), "!=", any-value)