[sheepdog] RFC: update protocol version or redesign struct sd_node

[Hitoshi Mitake]

At Tue, 14 Jan 2014 12:06:24 +0800,
Liu Yuan wrote:
> 
> On Tue, Jan 14, 2014 at 01:00:15PM +0900, Hitoshi Mitake wrote:
> > At Tue, 14 Jan 2014 11:34:03 +0800,
> > Liu Yuan wrote:
> > > 
> > > On Tue, Jan 14, 2014 at 12:00:33PM +0900, Hitoshi Mitake wrote:
> > > > 
> > > > The commit 9ea90be39e9d (sheep: use rbtree to manage struct sd_node)
> > > > changes struct sd_node. So the master branch isn't compatible with
> > > > v0.7.x because it affects the format of GET_NODE_LIST's returned
> > > > buffer. Every dog command which has a flag CMD_NEED_NODELIST cannot
> > > > work (users see nodes have IPv6 addresses).
> > > 
> > > Why you need v0.7.x dog to communite with latest sheep? There are many problems
> > > other than imcompatibility of struct sd_node
> > 
> > I don't need. Communication should be forbidden. Or if it is allowed,
> > the different format problem shuold be solved.
> > 
> > > 
> > > > The affection is very serious so I propose either of the below two
> > > > changes:
> > > 
> > > serious for what usage which can't be workaround and the only solution is to
> > > adopt following 2?
> > 
> > I'm not saying the 2nd one is the only solution. I don't have strong
> > opinions about this issue. Simply forbidding communication between
> > 0.7.x dog and the latest sheep is enough.
> 
> Oops, I misunderstood you and apologies for it.
> 
> > 
> > > 
> > > > 1. change protocol version and forbid communication between 0.7.x dog
> > > >    and the latest sheep
> > > > 
> > > > 2. change struct sd_node and exclude 	struct rb_node  rb; from the
> > > >    members
> > > > 
> > > > I think 2 would be a better option. sd_node is a data structure which
> > > 
> > > > can be exchanged via network and such a data structure shouldn't have
> > > > pointers as its members. Current sd_node is really ugly.
> > > 
> > > Talk is cheap, feel free to post a patch that fullfill current features:
> > > 1. allow 6k+ nodes (100k+ vnodes) manage effeciently (at least don't degrade)
> > > 2. don't break current usage (we already have a production cluster based on
> > >    current latest master, so your patch should work with current data without
> > >    re-format or any tricky upgrade. We want, at least for v0.8.x
> > >    series, we can get a smooth update for furture.
> > 
> > OK, I'll not follow the 2nd option.
> > 
> > > 
> > > There are many ugliness in sheep and also many existing problems. My advice is
> > > solve problems at first and don't invest time on something that changing core
> > > code just for something you think of beatiful. Chanign coring is always at risk
> > > to introduce new bugs and need time to stablize. I introduce rb_tree to manage
> > > vnodes because we have to use it to manage more than 10 thousands of vnodes, which
> > > old array approach can't handle well. So what is your motivation besides 'ugliness'?
> > 
> > Generally speaking, data structures which can be sent/recv network
> > shouldn't contain pointers. That's all.
> 
> Yes, I admited it was ugly but at the time I tried to remove 1024 max nodes, this
> is the most easy way to solve the problem and I couldn't manage to not include
> in the sd_node or sd_vnode for a tree management.
> 
> > Do you agree with incrementing protocol version?
> 
> Yes, I do agree.

OK, I'll send a patch for it later.

Thanks,
Hitoshi