Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

store: Correctly namespace etcd client #486

Merged
merged 1 commit into from Dec 12, 2017

Conversation

kshlm
Copy link
Member

@kshlm kshlm commented Dec 12, 2017

All etcd interfaces that needed to be namespaced (KV, Lease, Watcher,
Session) have been namespaced properly, for both remote store and
embedded stores.

As the etcd interfaces were not namespaced, some store operations
happened out of namespace. Also, elasticetcd unintentionally got
namespaced, causing some of its operations to happen in GD2s namespace.

All etcd interfaces that needed to be namespaced (KV, Lease, Watcher,
Session) have been namespaced properly, for both remote store and
embedded stores.

As the etcd interfaces were not namespaced, some store operations
happened out of namespace. Also, elasticetcd unintentionally got
namespaced, causing some of its operations to happen in GD2s namespace.
Copy link
Contributor

@prashanthpai prashanthpai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Have suggested one small change to a comment.

Checked the namespace now:

[ppai@gd2-1 ~]$ ETCDCTL_API=3 etcdctl get --prefix=true "" --endpoints=[127.0.0.1:2479]
elastic/election/3ee9604a56a00504
bffffede-1be5-4987-8989-929cb2255eaa
elastic/nominees/bffffede-1be5-4987-8989-929cb2255eaa
http://127.0.0.1:2480
elastic/volunteers/bffffede-1be5-4987-8989-929cb2255eaa
http://127.0.0.1:2480
gluster-9555d7aa-98c2-48cf-8d35-ccaf28181e85/alive/bffffede-1be5-4987-8989-929cb2255eaa

gluster-9555d7aa-98c2-48cf-8d35-ccaf28181e85/peers/bffffede-1be5-4987-8989-929cb2255eaa
{"ID":"bffffede-1be5-4987-8989-929cb2255eaa","Name":"gd2-1","Addresses":["127.0.0.1:24008"]}

lease := namespace.NewLease(oc.Lease, namespaceKey)
watcher := namespace.NewWatcher(oc.Watcher, namespaceKey)

// Create a new session (lease kept alive for the lifetime of a client)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment belongs just before the call to concurrency.NewSession

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. It's in the right place. The two blocks below together create the proper new session.

@prashanthpai prashanthpai merged commit 965fc14 into gluster:master Dec 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants