+1 to Hanifi's comments, I think it makes much more sense to have a number of sites where the operators are explicitly catching a checked OOM exception and either decide to handle it or produce a message like "Hash Aggregate does not support our of memory conditions". This would be particularly useful for debugging queries, as the user exception can provide context information about the current operation. This way users can have some idea about the part of their query that might be causing an excessive strain on system resources. I understand that there are also cases where operators competing for memory can make it a toss up to which will actually fail, but this would at least be a step to give more detailed information to users.
On Tue, Jun 30, 2015 at 1:28 PM, Hanifi GUNES <[email protected]> wrote: > I would propose throwing a checked exception encouraging explicit and > consistent handling of this event. Each sub-system has liberty to decide if > an OOM failure is fatal or non-fatal depending on its capabilities. Also if > at some point a sub-system needs to communicate with its callers via a > different mechanism such like using flags (boolean, enum etc) or raising an > unchecked exception that's still fine, just handle the exception. If there > is a need to suppress the checked exception that's fine too, just use a > helper method. > > Either way, returning *null* sounds problematic in many ways i) it is > implicit ii) unsafe iii) its handling logic is repetitive iv) it is > semantically unclean to make null mean something - even worse something > context specific. > > > -Hanifi > > 2015-06-30 12:23 GMT-07:00 Abdel Hakim Deneche <[email protected]>: > > > I guess that would fix the issue too. But we may still run into > situations > > where the caller will pass a flag to "mute" the exception and not handle > > the case anyway. > > > > If .buffer() unconditionally throws an exception, can't the caller, who > > wants to, just catch that and handle it properly ? > > > > On Tue, Jun 30, 2015 at 12:13 PM, Chris Westin <[email protected]> > > wrote: > > > > > No, but we should do something close to that. > > > > > > There are cases where the caller can handle the inability to get more > > > memory, and may be able to go to disk. However, you are correct that > > there > > > are many that can't handle an OOM, and that fail to check. > > > > > > Instead of unconditionally throwing OutOfMemoryRuntimeException, I > would > > > suggest that the buffer() call take a flag that indicates whether or > not > > it > > > should throw if it is unable to fulfill the request. This way, the call > > > sites that can handle an OOM can pass in the flag to return null, and > the > > > rest can pass in the flag value to throw, and not have to have any > > checking > > > code. > > > > > > > > > On Tue, Jun 30, 2015 at 12:06 PM, Abdel Hakim Deneche < > > > [email protected] > > > > wrote: > > > > > > > our current implementations of BufferAllocator.buffer(int, int) > returns > > > > null when it cannot allocate the buffer. > > > > > > > > But looking through the code, there are many places that don't check > if > > > the > > > > allocated buffer is null before trying to access it which will throw > a > > > > NullPointerException. > > > > > > > > ValueVectors' allocateNewSafe() seem to be the only place that handle > > the > > > > null in a specific manner. > > > > > > > > Should we update the allocators' implementation to throw an > > > > OutOfMemoryRuntimeException instead of returning null ? this has the > > > added > > > > benefit of displaying a proper out of memory error message to the > user. > > > > > > > > Thanks! > > > > > > > > -- > > > > > > > > Abdelhakim Deneche > > > > > > > > Software Engineer > > > > > > > > <http://www.mapr.com/> > > > > > > > > > > > > Now Available - Free Hadoop On-Demand Training > > > > < > > > > > > > > > > http://www.mapr.com/training?utm_source=Email&utm_medium=Signature&utm_campaign=Free%20available > > > > > > > > > > > > > > > > > > > > -- > > > > Abdelhakim Deneche > > > > Software Engineer > > > > <http://www.mapr.com/> > > > > > > Now Available - Free Hadoop On-Demand Training > > < > > > http://www.mapr.com/training?utm_source=Email&utm_medium=Signature&utm_campaign=Free%20available > > > > > >
