drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abdel Hakim Deneche <adene...@maprtech.com>
Subject Re: [DISCUSSION] should BufferAllocator.buffer() throw an OutOfMemoryException ?
Date Tue, 30 Jun 2015 20:53:03 GMT
+1 to Hanifi's

On Tue, Jun 30, 2015 at 1:38 PM, Jason Altekruse <altekrusejason@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 <hg@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 <adeneche@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 <
> chriswestin42@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 <
> > > > adeneche@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>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message