The language and store systems treat a JVM error such as OutOfMemoryError differently leading to the raw store protocol violation errors

• Key: DERBY-1732
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Sunitha Kambhampati
• Reporter: Daniel John Debrunner
• Component/s: SQL, Store
• Affects Version/s: 10.0.2.0, 10.0.2.1, 10.1.1.0, 10.1.2.1, 10.1.3.1, 10.2.1.6, 10.2.2.0, 10.3.1.4
• Fix Version/s: 10.2.2.0, 10.3.1.4

Description

Don't have the exact details, but remember noticing it a while ago. I think the store transaction context closes the transaction on such an error, while the language conneciton context just rollsback the transaction or the statement. I think the best and consistent approach would be to close the connection.

Comments

[Sunitha Kambhampati]

Notes:
On a JVM error, the cleanup at the store layer treats it as a session_severity error --
see RAMTransactionContext.cleanupOnError(), XactContext.cleanupOnError().
the transaction is aborted,destroyed, transaction context is pop'd from the context stack.

At the language side however:
GenericStatementContext (SC) will treat a java error to be similar to STATEMENT_SEVERITY.
GenericLanguageConnectionContex (LCC) will treat a java error to be of TRANSACTION_SEVERITY.

If an exception occurs, the contextManager.cleanupOnError is called to ensure corrective action can be taken. The context manager will unwind the contexts during cleanup in the reverse order they were placed in the global stack.

In ContextManager.cleanupOnError(),
-- A non StandardException (a jvm error) gets mapped to NO_APPLICABLE_SEVERITY here:
int errorSeverity = error instanceof StandardException ?
((StandardException) error).getSeverity() :
ExceptionSeverity.NO_APPLICABLE_SEVERITY;

-- Next, we walk down the stack calling cleanup on each context as long as if we reached the lastHandler or if we have reached end of stack.

So if a given context is the last handler for an error of some severity, then we stop there and do not call cleanup on the context's below it.

The derby1707 repro debugging shows that in this case - at prepare time for the statement, a NPE is thrown from GenericStatement.prepMinion. In the context stack,store contexts are first cleaned up and then the SC cleanup is called. Note: LCC is below in the stack and LCC cleanup does not get called since the SC.isLastHandler returns true.

Issue:
-- isLastHandler for GenericStatement is not right. JVM errors severity gets mapped to NO_APPLICABLE_SEVERITY in
ContextManager.cleanupOnError(). Thus on cleanup, for JVM error SC.isLastHandler can return true for some cases(when rollbackParentContext is false, and inUse is true). This is incorrect because for JVM errors, it is necessary to let the outer contexts know and take necessary corrective action.


This patch makes the following changes:
1. Make change to GenericStatementContext.isLastHandler() so it will return false for JVM errors thus allowing the outer contexts to take corrective action.
2. Store transaction context treats JVM errors as session severity. To ensure consistency, map severity for non StandardException instances to be SESSION_SEVERITY in GenericLanguageContext, and GenericStatementContext.

There is an existing DERBY-1528 which also results in a npe and then the raw store protocol violation error. With this change, if you run the repro (repro1528_npe), instead of a raw store protocol violation error, you will get a java.sql.SQLException 'No current exception'

Ran derbyall on ibm142/linux ok with 2 expected failures unrelated to this change. (NSInSameJVM and blobclob4BLOB)

I have been using the derby1707 repro without the fix for this jira for testing purposes. Are there any suggestions on how best to add a test for this case. One way is to simulate a java error using debug flags but I dont like that approach. Suggestions welcome. I can make the yet-to-be-fixed derby1528 repro as a test,but that will no longer test this fix when we eventually fix derby-1528.

Please take a look at this patch. I am new to this area, so would appreciate more eyes on this patch. Thanks.

[Daniel John Debrunner]

Patch looks good, I don't think there needs to be a test here. If the code had been correct in the first place there would never have been a test case because it is not functionality that is being developed.

[Daniel John Debrunner]

Patch Committed revision 453395 - Thanks Sunitha.

[Sunitha Kambhampati]

Verified fix in trunk. Thanks Dan for the review and commit.

[Rick Hillegas]

Re-opening to record porting to 10.2 branch

[Rick Hillegas]

Ported to 10.2 branch at subversion revision 480074.

[Sunitha Kambhampati]

Reopen to correct the assign to field.