- We would end up having to add it to almost everything everywhere Why would one propagate the checked exception for no reason? And why would one not propagate the exception for a good reason like robustness? I agree that one has to avoid propagating the checked exception for no reason however I support propagating it for a good reason.
The added benefit of raising a checked exception is reminding as well as enforcing devs to handle it and be more cautious about this particular event. I find this compile-time safety check invaluable for robustness. - Or constantly wrapping it with RuntimeException to get around that If it has to be done, I would recommend relying on a helper to do so. There is not much of man-work involved here. 2015-06-30 13:53 GMT-07:00 Abdel Hakim Deneche <adene...@maprtech.com>: > +1 to Hanifi's > > On Tue, Jun 30, 2015 at 1:38 PM, Jason Altekruse <altekruseja...@gmail.com > > > wrote: > > > +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 <h...@apache.org> 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 <adene...@maprtech.com > >: > > > > > > > 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 < > > chriswesti...@gmail.com> > > > > 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 < > > > > > adene...@maprtech.com > > > > > > 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 > > > > > > > > > > > > > > > > > > -- > > 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 > > >
