You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by mi...@apache.org on 2011/06/09 23:32:49 UTC
svn commit: r1134092 - in
/db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store:
access/btree/BTreePostCommit.java raw/data/BasePage.java
raw/data/StoredPage.java
Author: mikem
Date: Thu Jun 9 21:32:49 2011
New Revision: 1134092
URL: http://svn.apache.org/viewvc?rev=1134092&view=rev
Log:
DERBY-5258 btree post commit releases latch before committing/aborting purges, p
ossibly allowing other operation on page
backporting change 1132711 from trunk to 10.7 branch.
Fixing row level btree postcommit reclaim space to hold latch until end
of the internal transaction. Before this fix there was a very small window
(a few instructions) between the release of the latch and the commit of the
transaction where another transaction could access the page, insert rows,
and if a crash happens cause the undo of the purges of the reclaim space work
to fail.
It is proposed that this is what caused DERBY-5248, but without a repro it
can't be proved.
Modified:
db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java
db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java
db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java
Modified: db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java?rev=1134092&r1=1134091&r2=1134092&view=diff
==============================================================================
--- db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java (original)
+++ db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java Thu Jun 9 21:32:49 2011
@@ -207,7 +207,7 @@ class BTreePostCommit implements Service
this.access_factory.getAndNameTransaction(
contextMgr, AccessFactoryGlobals.SYS_TRANS_NAME);
- TransactionManager internal_xact = tc.getInternalTransaction();
+ TransactionManager internal_xact = tc.getInternalTransaction();
if (SanityManager.DEBUG)
{
@@ -215,7 +215,7 @@ class BTreePostCommit implements Service
System.out.println("starting internal xact\n");
}
- OpenBTree open_btree = null;
+ OpenBTree open_btree = null;
try
{
@@ -244,11 +244,8 @@ class BTreePostCommit implements Service
DataValueDescriptor[] shrink_key =
purgeCommittedDeletes(open_btree, this.page_number);
- // RESOLVE (mikem) - move this call when doing row level locking.
if (shrink_key != null)
doShrink(open_btree, shrink_key);
-
- open_btree.close();
}
catch (StandardException se)
{
@@ -277,8 +274,6 @@ class BTreePostCommit implements Service
purgeRowLevelCommittedDeletes(open_btree);
- open_btree.close();
-
}
catch (StandardException se2)
{
@@ -295,6 +290,15 @@ class BTreePostCommit implements Service
}
finally
{
+ if (open_btree != null)
+ open_btree.close();
+
+ // counting on this commit to release latches associated with
+ // row level purge, that have been left to prevent others from
+ // getting to purged pages before the commit. If latch is released
+ // early, other transactions could insert on the page which could
+ // prevent undo of the purges in case of a crash before the commit
+ // gets to the disk.
internal_xact.commit();
internal_xact.destroy();
}
@@ -330,8 +334,9 @@ class BTreePostCommit implements Service
* committed transactions (otherwise we could not have gotten the exclusive
* table lock).
* <p>
- * RESOLVE (mikem) - under row locking this routine must do more work to
- * determine a deleted row is a committed deleted row.
+ * This routine handles purging committed deletes while holding a table
+ * level exclusive lock. See purgeRowLevelCommittedDeletes() for row level
+ * purging.
*
* @param open_btree The btree already opened.
* @param pageno The page number of the page to look for committed deletes.
@@ -448,6 +453,12 @@ class BTreePostCommit implements Service
* If it succeeds, and since this transaction did not delete the row then
* the row must have been deleted by a transaction which has committed, so
* it is safe to purge the row. It then purges the row from the page.
+ * <p>
+ * The latch on the leaf page containing the purged rows must be kept until
+ * after the transaction has been committed or aborted in order to insure
+ * proper undo of the purges can take place. Otherwise another transaction
+ * could use the space freed by the purge and then prevent the purge from
+ * being able to undo.
*
* @param open_btree The already open btree, which has been locked with IX
* table lock, to use to get latch on page.
@@ -460,70 +471,68 @@ class BTreePostCommit implements Service
{
LeafControlRow leaf = null;
- try
+ // The following can fail, returning null, either if it can't get
+ // the latch or somehow the page requested no longer exists. In
+ // either case the post commit work will just skip it.
+ leaf = (LeafControlRow)
+ ControlRow.getNoWait(open_btree, page_number);
+ if (leaf == null)
+ return;
+
+ BTreeLockingPolicy btree_locking_policy =
+ open_btree.getLockingPolicy();
+
+ // The number records that can be reclaimed is:
+ // total recs - control row - recs_not_deleted
+ int num_possible_commit_delete =
+ leaf.page.recordCount() - 1 - leaf.page.nonDeletedRecordCount();
+
+ if (num_possible_commit_delete > 0)
{
- // The following can fail, returning null, either if it can't get
- // the latch or somehow the page requested no longer exists. In
- // either case the post commit work will just skip it.
- leaf = (LeafControlRow)
- ControlRow.getNoWait(open_btree, page_number);
- if (leaf == null)
- return;
-
- BTreeLockingPolicy btree_locking_policy =
- open_btree.getLockingPolicy();
-
- // The number records that can be reclaimed is:
- // total recs - control row - recs_not_deleted
- int num_possible_commit_delete =
- leaf.page.recordCount() - 1 - leaf.page.nonDeletedRecordCount();
+ DataValueDescriptor[] scratch_template =
+ open_btree.getRuntimeMem().get_template(
+ open_btree.getRawTran());
+
+ Page page = leaf.page;
+
- if (num_possible_commit_delete > 0)
+ // RowLocation column is in last column of template.
+ FetchDescriptor lock_fetch_desc =
+ RowUtil.getFetchDescriptorConstant(
+ scratch_template.length - 1);
+
+ // loop backward so that purges which affect the slot table
+ // don't affect the loop (ie. they only move records we
+ // have already looked at).
+ for (int slot_no = page.recordCount() - 1;
+ slot_no > 0;
+ slot_no--)
{
- DataValueDescriptor[] scratch_template =
- open_btree.getRuntimeMem().get_template(
- open_btree.getRawTran());
-
- Page page = leaf.page;
-
-
- // RowLocation column is in last column of template.
- FetchDescriptor lock_fetch_desc =
- RowUtil.getFetchDescriptorConstant(
- scratch_template.length - 1);
-
- // loop backward so that purges which affect the slot table
- // don't affect the loop (ie. they only move records we
- // have already looked at).
- for (int slot_no = page.recordCount() - 1;
- slot_no > 0;
- slot_no--)
+ if (page.isDeletedAtSlot(slot_no))
{
- if (page.isDeletedAtSlot(slot_no))
+ // try to get an exclusive lock on the row, if we can
+ // then the row is a committed deleted row and it is
+ // safe to purge it.
+ if (btree_locking_policy.lockScanCommittedDeletedRow(
+ open_btree, leaf, scratch_template,
+ lock_fetch_desc, slot_no))
{
- // try to get an exclusive lock on the row, if we can
- // then the row is a committed deleted row and it is
- // safe to purge it.
- if (btree_locking_policy.lockScanCommittedDeletedRow(
- open_btree, leaf, scratch_template,
- lock_fetch_desc, slot_no))
- {
- // the row is a committed deleted row, purge it.
- page.purgeAtSlot(slot_no, 1, true);
- // Tell scans positioned on this page to reposition
- // because the row they are positioned on may have
- // disappeared.
- page.setRepositionNeeded();
- }
+ // the row is a committed deleted row, purge it.
+ page.purgeAtSlot(slot_no, 1, true);
+ // Tell scans positioned on this page to reposition
+ // because the row they are positioned on may have
+ // disappeared.
+ page.setRepositionNeeded();
}
}
-
}
+
}
- finally
- {
- if (leaf != null)
- leaf.release();
- }
+
+ // need to maintain latch on leaf until xact is committed. The
+ // commit will clear the latch as part of releasing all
+ // locks/latches associated with a transaction.
+
+ return;
}
}
Modified: db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java?rev=1134092&r1=1134091&r2=1134092&view=diff
==============================================================================
--- db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java (original)
+++ db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java Thu Jun 9 21:32:49 2011
@@ -1362,10 +1362,11 @@ abstract class BasePage implements Page,
*/
public void unlatch() {
if (SanityManager.DEBUG) {
- SanityManager.ASSERT(isLatched());
+ SanityManager.ASSERT(isLatched(),
+ "unlatch() attempted on page that is not latched.");
}
- releaseExclusive();
+ releaseExclusive();
}
/**
@@ -1386,7 +1387,8 @@ abstract class BasePage implements Page,
/** @see Page#recordCount */
public final int recordCount() {
if (SanityManager.DEBUG) {
- SanityManager.ASSERT(isLatched());
+ SanityManager.ASSERT(
+ isLatched(), "page not latched on call to recordCount()");
}
return recordCount;
@@ -1427,9 +1429,14 @@ abstract class BasePage implements Page,
delCount++;
}
if (delCount != deletedCount)
- SanityManager.THROWASSERT("incorrect deleted row count. Should be: "
- + delCount + ", instead got: " + deletedCount
- + ", maxSlot = " + maxSlot + ", recordCount = " + recordCount);
+ {
+ SanityManager.THROWASSERT(
+ "incorrect deleted row count. Should be: " + delCount +
+ ", instead got: " + deletedCount +
+ ", maxSlot = " + maxSlot +
+ ", recordCount = " + recordCount +
+ "\npage = " + this);
+ }
}
return (recordCount - deletedCount);
Modified: db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java?rev=1134092&r1=1134091&r2=1134092&view=diff
==============================================================================
--- db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java (original)
+++ db/derby/code/branches/10.7/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java Thu Jun 9 21:32:49 2011
@@ -7034,7 +7034,8 @@ public class StoredPage extends CachedPa
public void logAction(LogInstant instant) throws StandardException
{
if (SanityManager.DEBUG) {
- SanityManager.ASSERT(isLatched());
+ SanityManager.ASSERT(isLatched(),
+ "logAction() executed on an unlatched page.");
}
if (rawDataOut == null)