incubator-blur-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Williams <>
Subject Re: reexamining combine
Date Wed, 12 Nov 2014 19:41:00 GMT
On Wed, Nov 12, 2014 at 9:03 AM, Aaron McCurry <> wrote:
> On Tue, Nov 11, 2014 at 3:56 PM, Tim Williams <> 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.


View raw message