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/06 19:57:28 UTC
svn commit: r1132711 - in
/db/derby/code/trunk/java/engine/org/apache/derby/impl/store:
access/btree/BTreePostCommit.java raw/data/BasePage.java
raw/data/StoredPage.java
Author: mikem
Date: Mon Jun 6 17:57:27 2011
New Revision: 1132711
URL: http://svn.apache.org/viewvc?rev=1132711&view=rev
Log:
DERBY-5258
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/trunk/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java
db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java
db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java
Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java?rev=1132711&r1=1132710&r2=1132711&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/store/access/btree/BTreePostCommit.java Mon Jun 6 17:57:27 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/trunk/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java?rev=1132711&r1=1132710&r2=1132711&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/BasePage.java Mon Jun 6 17:57:27 2011
@@ -1364,10 +1364,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();
}
/**
@@ -1388,7 +1389,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;
@@ -1429,9 +1431,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/trunk/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java?rev=1132711&r1=1132710&r2=1132711&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/store/raw/data/StoredPage.java Mon Jun 6 17:57:27 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)