Matt Lord

Results 130 comments of Matt Lord

> Thinking about this some more: the watchers are watching a specific key, right? So shouldn't we be passing `Version` which is key-specific, rather than the global `ModRevision`. For example,...

After further investigation (see my previous comment: https://github.com/vitessio/vitess/pull/15847#issuecomment-2096935831) I think that we should use `Revision/ModRevision` everywhere as the `topo.Version` value because that seems to be the only thing that etcdv3...

@rohit-nayak-ps Let's also look at an example since you seem to suspect that `Version` would give us better properties/semantics in determining order / sequence of operations and a key's relative...

@rohit-nayak-ps and @deepthi thank you for the reviews and discussion! I've now thoroughly documented the reason for this PR and the reasoning for the changes made. I've also now added...

> Thank you for the review @mattlord. I'll work on addressing them 👍. > > As far as the changes for running `PromoteReplica` in parallel, earlier we would call `PromoteReplica`...

> I don't like the idea of doing something that only works with etcd. ZooKeeper does not support this feature generally AFAIK. It's also worth noting that etcd2 is the...

@deepthi actually... it seems like ZooKeeper does support versions — there are virtually no docs so I didn't find anything, but... (after fixing the local example in: https://github.com/vitessio/vitess/pull/15933/commits/e5bef58d40044a80cea0f51808f1a26f1960b07e): ``` export...

Confirmed that ZooKeeper does not support getting a specific version of a node/key as it does not actually store multiple revisions/versions of a key: https://github.com/z-division/go-zookeeper/blob/v1.0.0/zk/conn.go#L1129-L1166 Rather the version in ZK...

Thank you for the review, @rohit-nayak-ps ! I've addressed your comments here: https://github.com/vitessio/vitess/pull/15933/commits/3ff727c2b95cc64f38634c438277654e791a18d7 Thank you for the nudge! I wasn't happy with that function either and it gave me the...

I’m afraid that I will have to mark this as can’t repeat as there’s no test case. Canceling a MoveTables workflow on a large table is something that happens all...