Skip to content

Conversation

@zuotingbing
Copy link

@zuotingbing zuotingbing commented Aug 2, 2017

What changes were proposed in this pull request?

Refer to Hive, ThriftCLIService methods such as ExecuteStatement are apparently capable of catching OOMs because they get wrapped in RTE by HiveSessionProxy.This shouldn't happen.
I create a PR to fix this bug in Spark also.

How was this patch tested?

Exist tests and manual tests

@gatorsmile
Copy link
Member

cc @liufengdb Is this PR in the right track?

import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.SynchronousQueue;
import java.util.concurrent.TimeUnit;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary change

import org.apache.hive.service.cli.thrift.ThriftCLIService;
import org.apache.hive.service.cli.thrift.ThriftHttpCLIService;

import java.util.Properties;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

java imports should be the first?

@liufengdb
Copy link

I actually did not get the motivation of this PR. HiveThriftServer2 can run independently or be started with a SQL context: https://github.com/apache/spark/pull/18812/files#diff-709404b0d3defeff035ef0c4f5a960e5L57. For the later, this OOM should be handled by the upper layer, right?

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@HyukjinKwon
Copy link
Member

gentle ping @zuotingbing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants