Hanifi GUNES wrote:
- 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.
+(1 times <some large number>)
Daniel
- 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 <[email protected]>:
+1 to Hanifi's
On Tue, Jun 30, 2015 at 1:38 PM, Jason Altekruse <[email protected]
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 <[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
--
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
--
Daniel Barclay
MapR Technologies
