Reason for nodes not including themselves in the dyn_seeds config?

[AlexBoyd]

I was wondering what the reasoning for requiring nodes not to list themselves in their dyn_seeds config?

If nodes were allowed to contain themselves it would slightly simplify the cluster wide config by allowing a single value that all nodes can share, rather than having separate config on a per node basis.

[shailesh33]

I completely agree. There is no doubt this will simplify the process. But I am guessing this was not done in the first place and thats why the config is still being inherited. It should not be a big deal to match one of the nodes in dyn_seeds to dyn_listen and listen. In fact, I know there is special handling in the code just because "self" is not added in the dyn_seeds. Sad but true.

There will be good change across the board for this: dynomite-manager and dynomite both. Honestly, our hands are more than full right now, but if you want to contribute, feel free.

On Wed, Mar 1, 2017 at 11:16 AM, AlexBoyd [email protected] wrote:

I was wondering what the reasoning for requiring nodes not to list themselves in their dyn_seeds config?

If nodes were allowed to contain themselves it would slightly simplify the cluster wide config by allowing a single value that all nodes can share, rather than having separate config on a per node basis.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Netflix/dynomite/issues/423, or mute the thread https://github.com/notifications/unsubscribe-auth/ALle5zXbfok1nEPkWsUbi8SYeFBG4US-ks5rhcQogaJpZM4MQFQa .

[AlexBoyd]

I took a look and it should be relatively easy to make this change.

The one issue is that it looks like the node doesn't use it's external hostname, but rather something like "0.0.0.0" when referring to itself. I could be wrong on that front, so I'd need to do a bit more digging.

If that is the case though I see 2 options to implementing this.

1. Match on dc / rack / token, and skip adding self to the dyn_seeds / peer list when there is already one provided via dyn_seeds for the rack / dc / token of the local node. or
2. Add a new config to toggle the behavior of adding the local node to the peer list.

I like option 1 a bit better as it doesn't require creating / setting a new config to prevent breaking backwards compatibility, but it is adding some hidden magic under the hood that would be unintuitive to someone trying to understand the dyn_seeds config without looking at the code.

If we had this change though, we could change the docs to specify that all nodes should be listed in dyn_seeds with a note that each node will insert itself into its local list if not present for backwards compatibility reasons.

[shailesh33]

I like the option 1 so that it is backward compatible. Eventually, we may want to error out when the self node is not there in the dyn_seeds to impose the config change, so that the code can be more clear.

On Wed, Mar 1, 2017 at 4:43 PM, AlexBoyd [email protected] wrote:

I took a look and it should be relatively easy to make this change.

The one issue is that it looks like the node doesn't use it's external hostname, but rather something like "0.0.0.0" when referring to itself. I could be wrong on that front, so I'd need to do a bit more digging.

If that is the case though I see 2 options to implementing this.

1. Match on dc / rack / token, and skip adding self to the dyn_seeds / peer list when there is already one provided via dyn_seeds for the rack / dc / token of the local node. or
2. Add a new config to toggle the behavior of adding the local node to the peer list.

I like option 1 a bit better as it doesn't require creating / setting a new config to prevent breaking backwards compatibility, but it is adding some hidden magic under the hood that would be unintuitive to someone trying to understand the dyn_seeds config without looking at the code.

If we had this change though, we could change the docs to specify that all nodes should be listed in dyn_seeds with a note that each node will insert itself into its local list if not present for backwards compatibility reasons.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Netflix/dynomite/issues/423#issuecomment-283519499, or mute the thread https://github.com/notifications/unsubscribe-auth/ALle594QxXM14h2_MHyIkfdYdDimCVxYks5rhhCdgaJpZM4MQFQa .

[shailesh33]

@AlexBoyd feel free to create a PR ;)