You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by prashanth-vasudev <gi...@git.apache.org> on 2016/12/22 00:43:39 UTC
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
GitHub user prashanth-vasudev opened a pull request:
https://github.com/apache/incubator-trafodion/pull/885
[TRAFODION-2414] enhance 8616 commit conflict error message to include tx conflict details
commit conflict error detail is available in the region.
This error is raised as a commit conflict exception and carried back to
transaction manager and further back to sql client.
Careful attention is given to make sure TM behaviour does not change
by making sure to the error codes continue to be returned along with
new exception strings.
Current set of change takes are of multi region transaction.
Additional change will be checked in to support single region transactions.
Error message looks like this:
>>commit work;
*** ERROR[8616] A conflict was detected during commit processing. Transaction has been aborted. Detail :org.trafodion.dtm.TransactionManagerException: org.apache.hadoop.hbase.coprocessor.transactional.CommitConflictException: java.io.IOException: Transaction [[transactionId: 17114 regionTX: false status: PENDING scan Size: 2 write Size: 1 startSQ: 2]] has scan which conflicts with [[transactionId: 1554366944090715 regionTX: true status: COMMITED scan Size: 2 write Size: 1 startSQ: 2 commitedSQ:2]]: region [TRAFODION.SCH.T116T6,,1482359816577.c4a43321f6a96035c3deeb0d85e55ccb.], scanRange[startRow: \x80\x00\x00\x01, endRow: \x80\x00\x00\x01], inserted row[\x80\x00\x00\x01]
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/prashanth-vasudev/incubator-trafodion tmErrMsg
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/885.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #885
----
commit 4a63d90d929a25940044d1a44f4e26dd6016c9cf
Author: Prashant Vasudev <pr...@esgyn.com>
Date: 2016-12-21T23:50:12Z
[TRAFODION-2414] enhance 8616 commit conflict error message to include tx conflict details.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/885#discussion_r94593238
--- Diff: core/sqf/src/tm/tmlib.cpp ---
@@ -956,13 +956,39 @@ short BEGINTX(int *pp_tag, int pv_timeout, int64 pv_type_flags)
return lv_error;
} //BEGINTX
+//--------------------------------------------------------------------
+//DEALLOCATE_ERR
+//
+//Purpose : Called subsequent to ENDTRANSACTION_ERR
+//Params : none
+//--------------------------------------------------------------------
+void DEALLOCATE_ERR(char *&errStr)
+{
+ if(errStr)
+ delete errStr;
--- End diff --
Should we set the errStr to NULL
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/885#discussion_r94592894
--- Diff: core/sqf/src/seatrans/tm/hbasetmlib2/javaobjectinterfacetm.cpp ---
@@ -478,3 +499,17 @@ void JavaObjectInterfaceTM::appendExceptionMessages(JNIEnv *jenv, jthrowable a_e
jenv->DeleteLocalRef(a_exception);
}
+short JavaObjectInterfaceTM::getExceptionErrorCode(JNIEnv *jenv, jthrowable a_exception)
+{
+ jshort errCode = JOI_OK;
+ if(a_exception != NULL)
+ {
+ //Only TransactionManagerException class has errorcode defined.
+ if(jenv->IsInstanceOf(a_exception, gTransactionManagerExceptionClass))
+ {
+ errCode =(jshort) jenv->CallShortMethod(a_exception,
+ gGetErrorCodeMethodID);
+ }
--- End diff --
Add exception handling code here or at least clear the exception and set the error code to zero
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by sbroeder <gi...@git.apache.org>.
Github user sbroeder commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/885#discussion_r94527876
--- Diff: core/sqf/src/seatrans/hbase-trx/src/main/java/org/apache/hadoop/hbase/client/transactional/TransactionManager.java ---
@@ -643,13 +644,18 @@ public CommitRequestResponse call(TrxRegionService instance) throws IOException
retry = true;
}
else if(result.size() == 1){
+ LOG.info("doPrepareX(MVCC), received result size: " + result.size() + " for transaction "
--- End diff --
An info comment here would result in 1 message per prepare. This should probably be a debug message or trace. and should also have the preceding if test to see if that level is enabled.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/885#discussion_r94589904
--- Diff: core/sqf/inc/cextdecs/cextdecs.h ---
@@ -289,6 +289,10 @@ _declspec(dllimport) short DNUMOUT
_declspec(dllimport) short ENDTRANSACTION();
+//errStr is allocated if errlen is not zero. Caller must deallocate errStr.
+//Rest of functionality same as ENDTRANSACTION();
+_declspec(dllimport) short ENDTRANSACTION_ERR(char *&errStr, int &errlen);
--- End diff --
Thanks @prashanth-vasudev for introducing another api to deallocate the error message. It is not clear why is it missing in the second time declaration in this file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/885
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/885#discussion_r93672871
--- Diff: core/sqf/src/seatrans/tm/hbasetmlib2/javaobjectinterfacetm.cpp ---
@@ -478,3 +514,23 @@ void JavaObjectInterfaceTM::appendExceptionMessages(JNIEnv *jenv, jthrowable a_e
jenv->DeleteLocalRef(a_exception);
}
+short JavaObjectInterfaceTM::getExceptionErrorCode(JNIEnv *jenv, jthrowable a_exception)
+{
+ jshort errCode = JOI_OK;
+ if(a_exception != NULL)
+ {
+ //Check to see if this exception has getErrorCode method.
+ //Only custom subclasses have errorcode defined.
--- End diff --
Consider caching the methodId or it is an expensive way to get the error code. Is this method called only when error is present
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/885#discussion_r93672570
--- Diff: core/sqf/src/seatrans/tm/hbasetmlib2/javaobjectinterfacetm.cpp ---
@@ -428,6 +428,42 @@ bool JavaObjectInterfaceTM::getExceptionDetails(JNIEnv *jenv)
return true;
}
--- End diff --
It would be better use the existing getExceptionDetails function to avoid duplication of code. You can either to choose to have default parameter or make the existing getExceptionDetails to call the new functions with retCode passed in
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request #885: [TRAFODION-2414] enhance 8616 commit ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/885#discussion_r93671479
--- Diff: core/sqf/inc/cextdecs/cextdecs.h ---
@@ -289,6 +289,10 @@ _declspec(dllimport) short DNUMOUT
_declspec(dllimport) short ENDTRANSACTION();
+//errStr is allocated if errlen is not zero. Caller must deallocate errStr.
+//Rest of functionality same as ENDTRANSACTION();
+_declspec(dllimport) short ENDTRANSACTION_ERR(char *&errStr, int &errlen);
--- End diff --
This would mean that the errStr is allocated by the TM functions using its own heap management scheme or the system memory. The caller wouldn't be aware of how to deallocate if it is anything other than system memory
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---