Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update UDFs and UDAs to handle failed allocations. #18

Merged

Conversation

Projects
None yet
3 participants
@timarmstrong
Copy link
Contributor

commented Aug 31, 2017

Impala may fail to allocate memory when under memory pressure. UDAs and
UDFs should be written to tolerate this. The examples here should
demonstrate best practices in handling these errors.

Testing:
Ran "make && ./build/uda-sample-test && ./build/udf-sample-test". The
test harness doesn't support testing failed allocations for now so I
did a couple of passes over the code to catch all the places where it
could potentially fail.

Change-Id: Ief21a733fa4281cd5178c58894e97e0af9751262

@timarmstrong timarmstrong requested a review from mjacobs Aug 31, 2017

@@ -146,6 +156,10 @@ void StringConcatUpdate(FunctionContext* context, const StringVal& str,
// separator.
int new_size = result->len + sep_ptr->len + str.len;
result->ptr = context->Reallocate(result->ptr, new_size);
if (result->ptr == NULL) {
// If the allocation fails, set the result to null and let Impala fail the query.
*result = StringVal::null();

This comment has been minimized.

Copy link
@sanjay-awat-guavus

sanjay-awat-guavus Sep 15, 2017

There should be a return after this statement otherwise the memcpy below might cause a crash

This comment has been minimized.

Copy link
@timarmstrong

timarmstrong Jan 18, 2018

Author Contributor

Done.

@timarmstrong

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2017

@michaelhkw any chance you could take a look at this, since you added some of the failed allocation handling originally.

@timarmstrong timarmstrong requested review from philz and removed request for mjacobs Jan 18, 2018

Update UDFs and UDAs to handle failed allocations.
Impala may fail to allocate memory when under memory pressure. UDAs and
UDFs should be written to tolerate this. The examples here should
demonstrate best practices in handling these errors.

Testing:
Ran "make && ./build/uda-sample-test && ./build/udf-sample-test". The
test harness doesn't support testing failed allocations for now so I
did a couple of passes over the code to catch all the places where it
could potentially fail.

Change-Id: Ief21a733fa4281cd5178c58894e97e0af9751262

@timarmstrong timarmstrong force-pushed the timarmstrong:handle-failed-allocations branch from dbe7eff to c5a52fc Jan 18, 2018

@timarmstrong timarmstrong merged commit 4959c24 into cloudera:master Mar 6, 2018

timarmstrong added a commit to timarmstrong/impala-udf-samples that referenced this pull request Mar 8, 2018

Merge pull request cloudera#18 from timarmstrong/handle-failed-alloca…
…tions

Update UDFs and UDAs to handle failed allocations.

Change-Id: I205e800ad23e07141cbf0c5a2c659ad67c89d180
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.