db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (DERBY-5947) Factor out common code from generated classes
Date Tue, 20 Nov 2012 15:02:59 GMT

     [ https://issues.apache.org/jira/browse/DERBY-5947?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Knut Anders Hatlen updated DERBY-5947:
--------------------------------------

    Attachment: d5947-5a-row-count-stats.diff

Attaching d5947-5a-row-count-stats.diff, which moves the row count
statistics from generated code to GenericPreparedStatement.

It was trickier than I had expected to get the tests to run cleanly
with the patch. I saw intermittent failures in the upgrade tests. They
failed because they triggered an assert in store when booting the
database for hard upgrade:

Caused by: org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED initSlotTable
consistency check failed:  slot 0 minimumRecordSize = 12 totalSpace = 12 recordPortionLength
= 8 reservedCount = 4
        at org.apache.derby.shared.common.sanity.SanityManager.THROWASSERT(SanityManager.java:162)
        at org.apache.derby.shared.common.sanity.SanityManager.THROWASSERT(SanityManager.java:147)
        at org.apache.derby.impl.store.raw.data.StoredPage.initSlotTable(StoredPage.java:2253)
        at org.apache.derby.impl.store.raw.data.StoredPage.initFromData(StoredPage.java:849)
        at org.apache.derby.impl.store.raw.data.CachedPage.setIdentity(CachedPage.java:213)
        at org.apache.derby.impl.services.cache.ConcurrentCache.find(ConcurrentCache.java:295)
        at org.apache.derby.impl.store.raw.data.FileContainer.getUserPage(FileContainer.java:2540)
        at org.apache.derby.impl.store.raw.data.FileContainer.getNextHeadPage(FileContainer.java:2858)
        at org.apache.derby.impl.store.raw.data.BaseContainer.getNextPage(BaseContainer.java:516)
        at org.apache.derby.impl.store.raw.data.BaseContainerHandle.getNextPage(BaseContainerHandle.java:364)
        at org.apache.derby.impl.store.access.conglomerate.GenericScanController.positionAtNextPage(GenericScanController.java:499)
        at org.apache.derby.impl.store.access.conglomerate.GenericScanController.fetchRows(GenericScanController.java:827)
        at org.apache.derby.impl.store.access.heap.HeapScan.fetchNext(HeapScan.java:238)
        at org.apache.derby.impl.sql.catalog.DataDictionaryImpl.clearSPSPlans(DataDictionaryImpl.java:4638)
        at org.apache.derby.impl.sql.catalog.DD_Version.handleMinorRevisionChange(DD_Version.java:555)
        at org.apache.derby.impl.sql.catalog.DD_Version.upgradeIfNeeded(DD_Version.java:250)
    (...)

It only happened occasionally, and it looked like the order in which
the test cases ran determined whether or not the test run succeeded. I
managed to isolate it further and reliably reproduce it using the
following steps:

1) Create an empty database with Derby 10.4.2.0 or earlier

2) Boot the database with trunk+patch (soft-upgrade) and create two
tables and a trigger:

    create table a(x int);
    create table b(y int);
    create trigger t after update on b
        referencing new as n for each row
        insert into a values (n.y);

3) Boot the database once with any Derby version 10.6.2.1 or earlier

4) Boot the database again with trunk, and see it fail with the above
mentioned assert failure

The failing assert was added in DERBY-4577. And indeed it only
reproduces if the downgrade step (3) is performed with a version that
suffers from DERBY-4577.

So what seems to be happening, is the following:

When the trigger is created in step (2) with trunk+patch, the trigger
plan is stored in a column in SYS.SYSSTATEMENTS. The plan is large
enough to make the row overflow to another page. When downgrading the
database in step (3), the old Derby version will go through all rows
in SYS.SYSSTATEMENTS and set the column that holds compiled plans to
NULL. Apparently, when shrinking the row, the old version runs into
the condition that caused DERBY-4577, and ends up setting aside too
little space for future expansion of the row. When subsequently
booting the database again with trunk, the assert in StoredPage
recognizes that this has happened, and fails.

If a version that includes the fix for DERBY-4577 is used in step 3,
the corruption doesn't happen, and trunk doesn't complain when booting
the database later.

Although I've only seen the assert failure with the patch, I don't
think it's a problem introduced by the patch. It's just that the new
and smaller trigger plans happen to make the SYSSTATEMENT rows
produced by these test cases of the exact right size to hit
DERBY-4577. Even without the patch, it might be possible to construct
a SYSSTATEMENTS table that trips over the same problem.

The patch works around this problem by forcing the problematic test
cases to run in a known good order. I've run the full upgrade test
suite successfully ten times with the workaround. Without the
workaround it would fail approximately every other run (with Java 7,
that is, where the test ordering varies between runs).


Finally, a description of what the patch does:

java/engine/org/apache/derby/iapi/reference/ClassName.java
java/engine/org/apache/derby/iapi/services/compiler/ClassBuilder.java
java/engine/org/apache/derby/iapi/services/compiler/MethodBuilder.java
java/engine/org/apache/derby/impl/services/bytecode/BCClass.java
java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java
java/engine/org/apache/derby/impl/sql/compile/ActivationClassBuilder.java
java/engine/org/apache/derby/impl/sql/compile/ExpressionClassBuilder.java
java/engine/org/apache/derby/impl/sql/compile/StatementNode.java

Removed code added by the 1a patch for manipulating static fields and
static initializers for the row count statistics.

java/engine/org/apache/derby/impl/sql/GenericPreparedStatement.java

Added fields to hold execution count, result set row count and stale
plan interval for a statement.

The fields were put in an inner class instead of directly inside
GenericPreparedStatement. This was done because triggers clone the
GenericPreparedStatement on each execution, so the statistics needed
to be shared among multiple GPS instances. For all other statements
than triggers, the statistics instance is private to one GPS instance.

Also, since the lifespan of a GPS instance is longer than that of the
generated activation class, code to reset the statistics on
recompilation was added.

org/apache/derby/iapi/sql/execute/ExecPreparedStatement.java

Interface methods that allowed calls to the new methods in
GenericPreparedStatement from BaseActivation.

java/engine/org/apache/derby/impl/sql/execute/BaseActivation.java

Removed inner class that held row count statistics.

Made shouldWeCheckRowCounts() and informOfRowCount() save the
statistics in the GenericPreparedStatement instead of the now removed
inner class.

java/engine/org/apache/derby/impl/sql/execute/ConstantActionActivation.java

Removed a method that's no longer used.

java/testing/org/apache/derbyTesting/functionTests/tests/upgradeTests/BasicSetup.java

Made the tests run in a fixed order, for reasons explained above.
                
> Factor out common code from generated classes
> ---------------------------------------------
>
>                 Key: DERBY-5947
>                 URL: https://issues.apache.org/jira/browse/DERBY-5947
>             Project: Derby
>          Issue Type: Improvement
>          Components: SQL
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>             Fix For: 10.10.0.0
>
>         Attachments: d5947-1a-remove-common-methods.diff, d5947-2a-execute-method.diff,
d5947-3a-init-rs.diff, d5947-4a-authorization.diff, d5947-5a-row-count-stats.diff, natural-join-after-3a.txt,
natural-join-decompiled.txt, values1-after-1a.txt, values1-after-2a.txt, values1-after-3a.txt,
values1-after-4a.txt, values1-decompiled.txt
>
>
> There's some code that's added to all classes generated by Derby's query compiler. For
example, there are three static fields that contain statistics used to check if the plan is
stale, and there are getter and setter methods for each of the three fields. The fields and
their accessor methods take up 468 bytes in every generated class.
> We should see if we can factor out some of this code so that there is a single shared
copy in BaseActivation. Advantages would be: less complicated byte-code generation, less memory
occupied by generated classes in the statement cache, smaller disk footprint for stored prepared
statements.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message