On Wed, Nov 12, 2014 at 9:03 AM, Aaron McCurry <[email protected]> wrote: > On Tue, Nov 11, 2014 at 3:56 PM, Tim Williams <[email protected]> wrote: > >> Thinking more about the current model, I'm a little concerned the >> current abstraction leaks an internal optimization. From the >> perspective of a command implementor the server combine is just an >> internal optimization? IOW, from a command perspective, it seems that >> we should be concerned about tables and shards and let >> 'server'-related things be plumbing underneath? >> >> We obviously want to well-define how exactly combine will be called >> (ie. for a given shard result, only ever once). I dunno, it smells a >> little funny right now, but then again, I know this method-smithing >> can be exhausting, just trying to get a feel for what others think? >> Anyone think it's worth going another round on this? >> > > I hear you on the abstraction/method sigs being a little weird for shard > server combines vs controller combines. I agree that I would like to let > the commands only have to deal with shard and table results. However I do > not want to remove an optimization path just for the sake of a cleaner api. > > I suppose we could change a bunch of the logic (again :-) ) and make the > combiner logic be an accumulator that can be serialized across shard and > controller servers. I haven't really thought this through but that may > allow us to make a single logic path for handling results. Thoughts?
I wouldn't hope to create a bunch of work, here's two simple ideas: 1) Change the in combine from Map<? extends Location<?>, T1> to Iterable<T1>... if you really need the context in your combine [mostly ill-advised anyway], you'd have to get it from the IndexContext in execute and handle passing it yourself. Little change across quite a few classes. 2) Accomplish the above by creating a new abstract class that hides the location info and have the docs encourage its use for most situations. Very little change. Thanks, --tim
