Hi Navneet,
I was always in favor of getting rid of AlwaysCreateBlock derived class.
This issue was raised long time back, but Craig insisted that we keep it.
I guess the PartitionAware class can't be an efficient drop in
replacement for the BlockPool class. The reason is that it checks for
each input tuple, which output partition should it be sent to. This is
an unnecessary step when we don't have partitioning enabled.
An alternative is something called PartitionPreserving derived class,
that Gerald wrote. It skips the above check that I mentioned.
Gerald, please feel free to correct if I didn't get it right.
On 06/15/2016 03:20 AM, Jignesh Patel wrote:
A huge +1 from me to get rid of dead code!
+CC: Gerald who also worked on this a while back, in case he has some input.
Thanks!
Jignesh
On Jun 14, 2016, at 11:36 PM, Navneet Potti <[email protected]> wrote:
Hi Harshad
I’m just kicking off this discussion based on a conversation I had with Jignesh
this morning. We have 3 different implementations of InsertDestination, and I
think we only really need one, the PartitionAwareBlockPoolInsertDestination.
The AlwaysCreateBlockInsertDestination seems to be entirely useless
practically. I’m guessing it’s a relic of early development. The normal
BlockPoolInsertDestination seems like a degenerate case of the PartitionAware
one, with a single partition.
If my understanding of the design is right, then we should refactor this code
to only have one InsertDestination. There is a lot of duplication of code and
complex logic in there. Anything we can do to simplify and clean up the code is
probably a good move.
Thoughts?
Cheers,
Navneet
--
Thanks,
Harshad
