You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by yo...@apache.org on 2017/02/10 16:16:23 UTC
lucene-solr:branch_6x: SOLR-10116: add more comments,
improve variable names in BlockCache
Repository: lucene-solr
Updated Branches:
refs/heads/branch_6x d009c4255 -> 5e5cbebde
SOLR-10116: add more comments, improve variable names in BlockCache
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/5e5cbebd
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/5e5cbebd
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/5e5cbebd
Branch: refs/heads/branch_6x
Commit: 5e5cbebde41b6f4df90f7d3733c6c28be3e7b756
Parents: d009c42
Author: yonik <yo...@apache.org>
Authored: Fri Feb 10 11:08:21 2017 -0500
Committer: yonik <yo...@apache.org>
Committed: Fri Feb 10 11:16:01 2017 -0500
----------------------------------------------------------------------
.../solr/store/blockcache/BlockCache.java | 41 +++++++++++++++++---
.../store/blockcache/BlockCacheLocation.java | 4 +-
2 files changed, 38 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/5e5cbebd/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java b/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java
index 3014550..7a5c67c 100644
--- a/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java
+++ b/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java
@@ -101,7 +101,18 @@ public class BlockCache {
metrics.blockCacheEviction.incrementAndGet();
metrics.blockCacheSize.decrementAndGet();
}
-
+
+ /**
+ * This is only best-effort... it's possible for false to be returned.
+ * The blockCacheKey is cloned before it is inserted into the map, so it may be reused by clients if desired.
+ *
+ * @param blockCacheKey the key for the block
+ * @param blockOffset the offset within the block
+ * @param data source data to write to the block
+ * @param offset offset within the source data array
+ * @param length the number of bytes to write.
+ * @return true if the block was cached/updated
+ */
public boolean store(BlockCacheKey blockCacheKey, int blockOffset,
byte[] data, int offset, int length) {
if (length + blockOffset > blockSize) {
@@ -115,12 +126,19 @@ public class BlockCache {
newLocation = true;
location = new BlockCacheLocation();
if (!findEmptyLocation(location)) {
+ // YCS: it looks like when the cache is full (a normal scenario), then two concurrent writes will result in one of them failing
+ // because no eviction is done first. The code seems to rely on leaving just a single block empty.
+ // TODO: simplest fix would be to leave more than one block empty
return false;
}
}
+
+ // YCS: I think this means that the block existed, but it is in the process of being
+ // concurrently removed. This flag is set in the releaseLocation eviction listener.
if (location.isRemoved()) {
return false;
}
+
int bankId = location.getBankId();
int bankOffset = location.getBlock() * blockSize;
ByteBuffer bank = getBank(bankId);
@@ -132,7 +150,15 @@ public class BlockCache {
}
return true;
}
-
+
+ /**
+ * @param blockCacheKey the key for the block
+ * @param buffer the target buffer for the read result
+ * @param blockOffset offset within the block
+ * @param off offset within the target buffer
+ * @param length the number of bytes to read
+ * @return true if the block was cached and the bytes were read
+ */
public boolean fetch(BlockCacheKey blockCacheKey, byte[] buffer,
int blockOffset, int off, int length) {
BlockCacheLocation location = cache.getIfPresent(blockCacheKey);
@@ -140,13 +166,14 @@ public class BlockCache {
return false;
}
if (location.isRemoved()) {
+ // location is in the process of being removed and the block may have already been reused by this point.
return false;
}
int bankId = location.getBankId();
- int offset = location.getBlock() * blockSize;
+ int bankOffset = location.getBlock() * blockSize;
location.touch();
ByteBuffer bank = getBank(bankId);
- bank.position(offset + blockOffset);
+ bank.position(bankOffset + blockOffset);
bank.get(buffer, off, length);
return true;
}
@@ -200,11 +227,13 @@ public class BlockCache {
+ "] got [" + buffer.length + "]");
}
}
-
+
+ /** Returns a new copy of the ByteBuffer for the given bank, so it's safe to call position() on w/o additional synchronization */
private ByteBuffer getBank(int bankId) {
return banks[bankId].duplicate();
}
-
+
+ /** returns the number of elements in the cache */
public int getSize() {
return cache.asMap().size();
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/5e5cbebd/solr/core/src/java/org/apache/solr/store/blockcache/BlockCacheLocation.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/store/blockcache/BlockCacheLocation.java b/solr/core/src/java/org/apache/solr/store/blockcache/BlockCacheLocation.java
index 36fb0a6..7792d90 100644
--- a/solr/core/src/java/org/apache/solr/store/blockcache/BlockCacheLocation.java
+++ b/solr/core/src/java/org/apache/solr/store/blockcache/BlockCacheLocation.java
@@ -35,6 +35,7 @@ public class BlockCacheLocation {
touch();
}
+ /** The block within the bank. This has no relationship to the blockId in BlockCacheKey */
public void setBlock(int block) {
this.block = block;
}
@@ -42,7 +43,8 @@ public class BlockCacheLocation {
public void setBankId(int bankId) {
this.bankId = bankId;
}
-
+
+ /** The block within the bank. This has no relationship to the blockId in BlockCacheKey */
public int getBlock() {
return block;
}