Skip to content
This repository has been archived by the owner on Jun 7, 2021. It is now read-only.

[TRAFODION-2821] Trafodion core code base needs to be thread safe #1493

Merged
merged 2 commits into from Mar 23, 2018

Conversation

selvaganesang
Copy link
Contributor

@selvaganesang selvaganesang commented Mar 22, 2018

Java exceptions thrown while calling the java methods from JNI in
Trafodion are stored in the current context. However in a multi-threaded
ESP environment, these exceptions should be stored in thread specific
variable to enable error handling to be thread safe. Otherwise, the JNI
errors could be overwritten by the JNI errors from another thread.

Also streamlined JNI error handling by extending the getExceptionDetails()
method to log the errors also.

Incorporated error handling in SequenceFileReader JNI methods.

Java exceptions thrown while calling the java methods from JNI in
Trafodion are stored in the current context. However in a multi-threaded
ESP environment, these exceptions should be stored in thread specific
variable to enable error handling to be thread safe. Otherwise, the JNI
errors could be overwritten by the JNI errors from another thread.

Also streamlined JNI error handling by extending the getExceptionDetails()
methods to log the errors also.

Incorporated error handling in SequenceFileReader JNI methods.
@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2511/

@Traf-Jenkins
Copy link

@selvaganesang
Copy link
Contributor Author

@robertamarton Can you please review this change

@zellerh
Copy link
Contributor

zellerh commented Mar 22, 2018

+1
Looks good to me.

Copy link
Contributor

@DaveBirdsall DaveBirdsall left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. I think there is one place where a PopLocalFrame call was removed that perhaps should not have been.

getExceptionDetails();
logError(CAT_SQL_HBASE, __FILE__, __LINE__);
logError(CAT_SQL_HBASE, "HBaseClient_JNI::releaseHTableClient()", getLastError());
jenv_->PopLocalFrame(NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to delete the jenv_->PopLocalFrame call here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I will add it back in my next check-in

Fixes as per review comments and missed out changes in the commit 073cf68
@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2519/

@Traf-Jenkins
Copy link

@sureshsubbiah
Copy link
Contributor

+1. Looks good to me.

@asfgit asfgit merged commit cc97254 into apache:master Mar 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
6 participants