ignite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sboi...@apache.org
Subject [08/50] [abbrv] ignite git commit: ignite-db-x - null locking fixes
Date Mon, 10 Oct 2016 12:13:04 GMT
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 <sergi.vladykin@gmail.com>
Authored: Mon Sep 26 22:10:40 2016 +0300
Committer: Sergi Vladykin <sergi.vladykin@gmail.com>
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<L, T extends L> 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<L> io = io(buf);
@@ -983,7 +983,7 @@ public abstract class BPlusTree<L, T extends L> 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<L> io = io(buf);
@@ -2213,6 +2213,8 @@ public abstract class BPlusTree<L, T extends L> 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<L, T extends L> 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);


Mime
View raw message