2016-06-07 13:10 GMT+02:00 'Brian Foley' via ganeti-devel < ganeti-devel@googlegroups.com>:
> Hi Iustin, > > I'm half inclined to think that if we're going to change the type of UUID > we should do it 'properly', ie change to a strict, unpacked 16-byte array > that implements Eq, Show, Read (and maybe Hashable too) and use it > consistently everywhere. > 16-byte - does that mean you favour Data.Text over ByteString? I was thinking that way as well (due to ease of using one underlying type throughout), but we will pay 2N compared to bytes on disk. And at least for UUIDs, we don't do any string-related processing on them (substrings, casing, etc.) so we could save some memory there. On the other hand, Data.ByteString.Short is a newer addition, so using it might have some small logistical drawbacks, plus that (very rarely) we do have conversions. Hmm… I'd also be tempted to introduce a type that lets us distinguish between > UUIDs for different Ganeti object types, eg Disks, Nodes, Instances, NICs > etc. > Interesting. On one hand, they are the same thing (UUIDs), and we do process them together at one point (although badly - in getAllIDs). On the other hand, they do refer to different things :) Were you thinking of documentation only (type aliases), or hard separation (newtypes)? Efficiency-wise, yeah, we do have the problem where we don't have a suite > of microbenchmarks at the moment, and we don't really know which functions > are critical to overall performance. As you say, it's entirely possible > that improving performance in one area might just cause problems somewhere > else. > > Our testing approach at the moment is to take a relatively heavy workload > -- a parallel 4 node evacuate on a 80 node/1200 instance cluster -- and to > see what problems that exposes, but that's far from perfect. > > For what it's worth, I found that getNodeInstances shows up high in the > profile when doing a RAPI /2/nodes requests, and it would also matter for > ReqNodeInstances queries to confd. Otherwise it doesn't seem to be > something that's heavily used. > Ack to the above. I think I agree with you: we shouldn't make any large-scale changes on > anything but master because there's too much potential to introduce > performance regressions and we don't have a good way to measure them. > Instead, we're focusing on point fixes, and trying to get 2.16 released, > since it has a bunch of random efficiency improvements relative to 2.15, > improves KVM support, and allows configurable SSH key types/sizes. > Cool. Let's see where the discussion regarding Data.Text goes (anybody else wants to chime in), and we can proceed (in master) along those lines. thanks for the feedback! iustin
