Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id EAF68200BAD for ; Mon, 10 Oct 2016 14:12:59 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id E97CE160B05; Mon, 10 Oct 2016 12:12:59 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id B0210160AF9 for ; Mon, 10 Oct 2016 14:12:58 +0200 (CEST) Received: (qmail 47219 invoked by uid 500); 10 Oct 2016 12:12:57 -0000 Mailing-List: contact commits-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list commits@ignite.apache.org Received: (qmail 46906 invoked by uid 99); 10 Oct 2016 12:12:57 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 10 Oct 2016 12:12:57 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 368FBE698F; Mon, 10 Oct 2016 12:12:57 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: sboikov@apache.org To: commits@ignite.apache.org Date: Mon, 10 Oct 2016 12:13:04 -0000 Message-Id: <7d62a9fb2ca94b8c8de31d0360721542@git.apache.org> In-Reply-To: <1c2b871572b547e0bad93846577cf97b@git.apache.org> References: <1c2b871572b547e0bad93846577cf97b@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [08/50] [abbrv] ignite git commit: ignite-db-x - null locking fixes archived-at: Mon, 10 Oct 2016 12:13:00 -0000 ignite-db-x - null locking fixes Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/86925b14 Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/86925b14 Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/86925b14 Branch: refs/heads/ignite-gg-8-io2-selNow Commit: 86925b14df5a857dd2faacbe4b4febba855545a2 Parents: 4e2fe38 Author: Sergi Vladykin Authored: Mon Sep 26 22:10:40 2016 +0300 Committer: Sergi Vladykin Committed: Mon Sep 26 22:10:40 2016 +0300 ---------------------------------------------------------------------- .../cache/database/CacheDataRowAdapter.java | 4 +- .../cache/database/freelist/PagesList.java | 56 +++++++++++--------- .../cache/database/tree/BPlusTree.java | 8 ++- 3 files changed, 38 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/86925b14/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/CacheDataRowAdapter.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/CacheDataRowAdapter.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/CacheDataRowAdapter.java index dd0358c..b5babc4 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/CacheDataRowAdapter.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/CacheDataRowAdapter.java @@ -89,9 +89,9 @@ public class CacheDataRowAdapter implements CacheDataRow { do { try (Page page = page(pageId(nextLink), cctx)) { - ByteBuffer buf = page.getForRead(); + ByteBuffer buf = page.getForRead(); // Non-empty data page must not be recycled. - assert buf != null: nextLink; // Non-empty data page must not be recycled. + assert buf != null: nextLink; try { DataPageIO io = DataPageIO.VERSIONS.forPage(buf); http://git-wip-us.apache.org/repos/asf/ignite/blob/86925b14/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/freelist/PagesList.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/freelist/PagesList.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/freelist/PagesList.java index bfbc68f..8156dd2 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/freelist/PagesList.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/freelist/PagesList.java @@ -149,7 +149,9 @@ public abstract class PagesList extends DataStructure { while (nextPageId != 0) { try (Page page = page(nextPageId)) { - ByteBuffer buf = readLock(page); + ByteBuffer buf = readLock(page); // No concurrent recycling on init. + + assert buf != null; try { PagesListMetaIO io = PagesListMetaIO.VERSIONS.forPage(buf); @@ -460,7 +462,7 @@ public abstract class PagesList extends DataStructure { long pageId = tail.tailId; try (Page page = page(pageId)) { - ByteBuffer buf = readLock(page); + ByteBuffer buf = readLock(page); // No correctness guaranties. try { PagesListNodeIO io = PagesListNodeIO.VERSIONS.forPage(buf); @@ -499,7 +501,7 @@ public abstract class PagesList extends DataStructure { long tailId = stripe.tailId; try (Page tail = page(tailId)) { - ByteBuffer buf = writeLockPage(tail, bucket, lockAttempt++); + ByteBuffer buf = writeLockPage(tail, bucket, lockAttempt++); // Explicit check. if (buf == null) continue; @@ -507,9 +509,6 @@ public abstract class PagesList extends DataStructure { boolean ok = false; try { - if (PageIO.getPageId(buf) != tailId) - continue; // Page was recycled -> retry. - PagesListNodeIO io = PageIO.getPageIO(buf); ok = bag != null ? @@ -627,7 +626,9 @@ public abstract class PagesList extends DataStructure { long nextId = allocatePage(null); try (Page next = page(nextId)) { - ByteBuffer nextBuf = writeLock(next); + ByteBuffer nextBuf = writeLock(next); // Newly allocated page. + + assert nextBuf != null; try { setupNextPage(io, pageId, buf, nextId, nextBuf); @@ -702,7 +703,9 @@ public abstract class PagesList extends DataStructure { if (idx == -1) { // Attempt to add page failed: the node page is full. try (Page next = page(nextId)) { - ByteBuffer nextBuf = writeLock(next); + ByteBuffer nextBuf = writeLock(next); // Page from reuse bag can't be concurrently recycled. + + assert nextBuf != null; if (locked == null) { lockedBufs = new ArrayList<>(2); @@ -796,7 +799,7 @@ public abstract class PagesList extends DataStructure { } } - return lockAttempt < TRY_LOCK_ATTEMPTS ? null : writeLock(page); + return lockAttempt < TRY_LOCK_ATTEMPTS ? null : writeLock(page); // Must be explicitly checked further. } /** @@ -817,7 +820,7 @@ public abstract class PagesList extends DataStructure { long tailId = stripe.tailId; try (Page tail = page(tailId)) { - ByteBuffer tailBuf = writeLockPage(tail, bucket, lockAttempt++); + ByteBuffer tailBuf = writeLockPage(tail, bucket, lockAttempt++); // Explicit check. if (tailBuf == null) continue; @@ -825,9 +828,6 @@ public abstract class PagesList extends DataStructure { boolean dirty = false; try { - if (getPageId(tailBuf) != tailId) - continue; - PagesListNodeIO io = PagesListNodeIO.VERSIONS.forPage(tailBuf); if (io.getNextId(tailBuf) != 0) @@ -917,14 +917,14 @@ public abstract class PagesList extends DataStructure { long recycleId = 0L; - ByteBuffer buf = writeLock(page); + ByteBuffer buf = writeLock(page); // Explicit check. + + if (buf == null) + return false; boolean rmvd = false; try { - if (getPageId(buf) != pageId) - return false; - PagesListNodeIO io = PagesListNodeIO.VERSIONS.forPage(buf); rmvd = io.removePage(buf, dataPageId); @@ -1014,20 +1014,24 @@ public abstract class PagesList extends DataStructure { try (Page next = nextId == 0L ? null : page(nextId)) { boolean write = false; - ByteBuffer nextBuf = next == null ? null : writeLock(next); - ByteBuffer buf = writeLock(page); + ByteBuffer nextBuf = next == null ? null : writeLock(next); // Explicit check. + ByteBuffer buf = writeLock(page); // Explicit check. - try { - if (getPageId(buf) != pageId) - return 0L; // Someone has merged or taken our empty page concurrently. Nothing to do here. + if (buf == null) { + if (nextBuf != null) // Unlock next page if needed. + writeUnlock(next, nextBuf, false); + + return 0L; // Someone has merged or taken our empty page concurrently. Nothing to do here. + } + try { PagesListNodeIO io = PagesListNodeIO.VERSIONS.forPage(buf); if (!io.isEmpty(buf)) return 0L; // No need to merge anymore. // Check if we see a consistent state of the world. - if (io.getNextId(buf) == nextId) { + if (io.getNextId(buf) == nextId && (nextId == 0L) == (nextBuf == null)) { long recycleId = doMerge(pageId, page, buf, io, next, nextId, nextBuf, bucket); write = true; @@ -1112,11 +1116,11 @@ public abstract class PagesList extends DataStructure { ByteBuffer nextBuf) throws IgniteCheckedException { try (Page prev = page(prevId)) { - ByteBuffer prevBuf = writeLock(prev); + ByteBuffer prevBuf = writeLock(prev); // No check, we keep a reference. - try { - assert getPageId(prevBuf) == prevId; // Because we keep a reference. + assert prevBuf != null; + try { PagesListNodeIO prevIO = PagesListNodeIO.VERSIONS.forPage(prevBuf); PagesListNodeIO nextIO = PagesListNodeIO.VERSIONS.forPage(nextBuf); http://git-wip-us.apache.org/repos/asf/ignite/blob/86925b14/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/tree/BPlusTree.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/tree/BPlusTree.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/tree/BPlusTree.java index 64809c9..475176a 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/tree/BPlusTree.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/database/tree/BPlusTree.java @@ -922,7 +922,7 @@ public abstract class BPlusTree extends DataStructure { */ private void validateDownKeys(long pageId, L minRow) throws IgniteCheckedException { try (Page page = page(pageId)) { - ByteBuffer buf = readLock(page); + ByteBuffer buf = readLock(page); // No correctness guaranties. try { BPlusIO io = io(buf); @@ -983,7 +983,7 @@ public abstract class BPlusTree extends DataStructure { */ private L getGreatestRowInSubTree(long pageId) throws IgniteCheckedException { try (Page page = page(pageId)) { - ByteBuffer buf = readLock(page); + ByteBuffer buf = readLock(page); // No correctness guaranties. try { BPlusIO io = io(buf); @@ -2213,6 +2213,8 @@ public abstract class BPlusTree extends DataStructure { ByteBuffer fwdBuf = writeLock(fwd); // Initial write, no need to check for concurrent modification. + assert fwdBuf != null; + try { // Never write full forward page, because it is known to be new. fwd.fullPageWalRecordPolicy(Boolean.FALSE); @@ -2258,6 +2260,8 @@ public abstract class BPlusTree extends DataStructure { ByteBuffer newRootBuf = writeLock(newRoot); // Initial write. + assert newRootBuf != null; + try { // Never write full new root page, because it is known to be new. newRoot.fullPageWalRecordPolicy(Boolean.FALSE);