You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by be...@apache.org on 2015/09/10 14:55:59 UTC
[1/3] cassandra git commit: 8630: fixed coverity defects
Repository: cassandra
Updated Branches:
refs/heads/cassandra-3.0 9f0509ee3 -> 967a2cfe1
refs/heads/trunk 55b6e8a07 -> 21d4a963d
8630: fixed coverity defects
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/967a2cfe
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/967a2cfe
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/967a2cfe
Branch: refs/heads/cassandra-3.0
Commit: 967a2cfe179548835d5e8c1640889420ce0d40ce
Parents: 9f0509e
Author: Stefania Alborghetti <st...@datastax.com>
Authored: Mon Sep 7 09:44:46 2015 +0800
Committer: Benedict Elliott Smith <be...@apache.org>
Committed: Thu Sep 10 13:55:06 2015 +0100
----------------------------------------------------------------------
.../cassandra/hints/ChecksummedDataInput.java | 2 +-
.../compress/CompressedRandomAccessReader.java | 30 +++----
.../cassandra/io/util/RandomAccessReader.java | 91 +++++++++-----------
.../cassandra/utils/memory/BufferPool.java | 2 +-
4 files changed, 56 insertions(+), 69 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java b/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
index 543f14e..95ea256 100644
--- a/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
+++ b/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
@@ -137,7 +137,7 @@ public final class ChecksummedDataInput extends RandomAccessReader.RandomAccessR
private void updateCrc()
{
- if (crcPosition == buffer.position() | crcUpdateDisabled)
+ if (crcPosition == buffer.position() || crcUpdateDisabled)
return;
assert crcPosition >= 0 && crcPosition < buffer.position();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java b/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
index 0242871..7294923 100644
--- a/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
+++ b/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
@@ -48,29 +48,13 @@ public class CompressedRandomAccessReader extends RandomAccessReader
protected CompressedRandomAccessReader(Builder builder)
{
- super(builder.initializeBuffers(false));
+ super(builder);
this.metadata = builder.metadata;
this.checksum = metadata.checksumType.newInstance();
- initializeBuffer();
- }
-
- @Override
- protected int getBufferSize(RandomAccessReader.Builder builder)
- {
- // this is the chunk data length, throttling is OK with this
- return builder.bufferSize;
- }
-
- @Override
- protected void initializeBuffer()
- {
- buffer = allocateBuffer(bufferSize);
- buffer.limit(0);
-
if (regions == null)
{
- compressed = allocateBuffer(metadata.compressor().initialCompressedBufferLength(metadata.chunkLength()));
+ compressed = allocateBuffer(metadata.compressor().initialCompressedBufferLength(metadata.chunkLength()), bufferType);
checksumBytes = ByteBuffer.wrap(new byte[4]);
}
}
@@ -110,7 +94,7 @@ public class CompressedRandomAccessReader extends RandomAccessReader
if (compressed.capacity() < chunk.length)
{
BufferPool.put(compressed);
- compressed = allocateBuffer(chunk.length);
+ compressed = allocateBuffer(chunk.length, bufferType);
}
else
{
@@ -278,6 +262,14 @@ public class CompressedRandomAccessReader extends RandomAccessReader
}
@Override
+ protected ByteBuffer createBuffer()
+ {
+ buffer = allocateBuffer(bufferSize, bufferType);
+ buffer.limit(0);
+ return buffer;
+ }
+
+ @Override
public RandomAccessReader build()
{
return new CompressedRandomAccessReader(this);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/io/util/RandomAccessReader.java b/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
index e9b0ee4..43589d0 100644
--- a/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
+++ b/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
@@ -33,9 +33,13 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
// The default buffer size when the client doesn't specify it
public static final int DEFAULT_BUFFER_SIZE = 4096;
- // The maximum buffer size when the limiter is not null, i.e. when throttling
- // is enabled. This is required to avoid aquiring permits that are too large.
- public static final int MAX_THROTTLED_BUFFER_SIZE = 1 << 16; // 64k
+ // The maximum buffer size, we will never buffer more than this size. Further,
+ // when the limiter is not null, i.e. when throttling is enabled, we read exactly
+ // this size, since when throttling the intention is to eventually read everything,
+ // see CASSANDRA-8630
+ // NOTE: this size is chosen both for historical consistency, as a reasonable upper bound,
+ // and because our BufferPool currently has a maximum allocation size of this.
+ public static final int MAX_BUFFER_SIZE = 1 << 16; // 64k
// the IO channel to the file, we do not own a reference to this due to
// performance reasons (CASSANDRA-9379) so it's up to the owner of the RAR to
@@ -57,7 +61,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
protected final int bufferSize;
// the buffer type for buffered readers
- private final BufferType bufferType;
+ protected final BufferType bufferType;
// offset from the beginning of the file
protected long bufferOffset;
@@ -67,42 +71,18 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
protected RandomAccessReader(Builder builder)
{
- super(null);
+ super(builder.createBuffer());
this.channel = builder.channel;
this.regions = builder.regions;
this.limiter = builder.limiter;
this.fileLength = builder.overrideLength <= 0 ? builder.channel.size() : builder.overrideLength;
- this.bufferSize = getBufferSize(builder);
+ this.bufferSize = builder.bufferSize;
this.bufferType = builder.bufferType;
-
- if (builder.bufferSize <= 0)
- throw new IllegalArgumentException("bufferSize must be positive");
-
- if (builder.initializeBuffers)
- initializeBuffer();
- }
-
- protected int getBufferSize(Builder builder)
- {
- if (builder.limiter == null)
- return builder.bufferSize;
-
- // limit to ensure more accurate throttling
- return Math.min(MAX_THROTTLED_BUFFER_SIZE, builder.bufferSize);
- }
-
- protected void initializeBuffer()
- {
- if (regions == null)
- buffer = allocateBuffer(bufferSize);
- else
- buffer = regions.floor(0).buffer.duplicate();
-
- buffer.limit(0);
+ this.buffer = builder.buffer;
}
- protected ByteBuffer allocateBuffer(int size)
+ protected static ByteBuffer allocateBuffer(int size, BufferType bufferType)
{
return BufferPool.get(size, bufferType).order(ByteOrder.BIG_ENDIAN);
}
@@ -396,35 +376,56 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
// The type of the buffer for buffered readers
public BufferType bufferType;
+ // The buffer
+ public ByteBuffer buffer;
+
// The mmap segments for mmap readers
public MmappedRegions regions;
// An optional limiter that will throttle the amount of data we read
public RateLimiter limiter;
- public boolean initializeBuffers;
-
public Builder(ChannelProxy channel)
{
this.channel = channel;
this.overrideLength = -1L;
- this.bufferSize = getBufferSize(DEFAULT_BUFFER_SIZE);
+ this.bufferSize = DEFAULT_BUFFER_SIZE;
this.bufferType = BufferType.OFF_HEAP;
this.regions = null;
this.limiter = null;
- this.initializeBuffers = true;
}
/** The buffer size is typically already page aligned but if that is not the case
- * make sure that it is a multiple of the page size, 4096.
+ * make sure that it is a multiple of the page size, 4096. Also limit it to the maximum
+ * buffer size unless we are throttling, in which case we may as well read the maximum
+ * directly since the intention is to read the full file, see CASSANDRA-8630.
* */
- private static int getBufferSize(int size)
+ private void setBufferSize()
{
- if ((size & ~4095) != size)
+ if (limiter != null)
+ {
+ bufferSize = MAX_BUFFER_SIZE;
+ return;
+ }
+
+ if ((bufferSize & ~4095) != bufferSize)
{ // should already be a page size multiple but if that's not case round it up
- size = (size + 4095) & ~4095;
+ bufferSize = (bufferSize + 4095) & ~4095;
}
- return size;
+
+ bufferSize = Math.min(MAX_BUFFER_SIZE, bufferSize);
+ }
+
+ protected ByteBuffer createBuffer()
+ {
+ setBufferSize();
+
+ buffer = regions == null
+ ? allocateBuffer(bufferSize, bufferType)
+ : regions.floor(0).buffer.duplicate();
+
+ buffer.limit(0);
+ return buffer;
}
public Builder overrideLength(long overrideLength)
@@ -441,7 +442,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
if (bufferSize <= 0)
throw new IllegalArgumentException("bufferSize must be positive");
- this.bufferSize = getBufferSize(bufferSize);
+ this.bufferSize = bufferSize;
return this;
}
@@ -463,12 +464,6 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
return this;
}
- public Builder initializeBuffers(boolean initializeBuffers)
- {
- this.initializeBuffers = initializeBuffers;
- return this;
- }
-
public RandomAccessReader build()
{
return new RandomAccessReader(this);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/utils/memory/BufferPool.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/utils/memory/BufferPool.java b/src/java/org/apache/cassandra/utils/memory/BufferPool.java
index a64cfb0..f972059 100644
--- a/src/java/org/apache/cassandra/utils/memory/BufferPool.java
+++ b/src/java/org/apache/cassandra/utils/memory/BufferPool.java
@@ -44,7 +44,7 @@ import org.apache.cassandra.utils.concurrent.Ref;
*/
public class BufferPool
{
- /** The size of a page aligned buffer, 64kbit */
+ /** The size of a page aligned buffer, 64KiB */
static final int CHUNK_SIZE = 64 << 10;
@VisibleForTesting
[3/3] cassandra git commit: Merge branch 'cassandra-3.0' into trunk
Posted by be...@apache.org.
Merge branch 'cassandra-3.0' into trunk
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/21d4a963
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/21d4a963
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/21d4a963
Branch: refs/heads/trunk
Commit: 21d4a963d0ca62563ca19e00d47d383aec9faa71
Parents: 55b6e8a 967a2cf
Author: Benedict Elliott Smith <be...@apache.org>
Authored: Thu Sep 10 13:55:40 2015 +0100
Committer: Benedict Elliott Smith <be...@apache.org>
Committed: Thu Sep 10 13:55:40 2015 +0100
----------------------------------------------------------------------
.../cassandra/hints/ChecksummedDataInput.java | 2 +-
.../compress/CompressedRandomAccessReader.java | 30 +++----
.../cassandra/io/util/RandomAccessReader.java | 91 +++++++++-----------
.../cassandra/utils/memory/BufferPool.java | 2 +-
4 files changed, 56 insertions(+), 69 deletions(-)
----------------------------------------------------------------------
[2/3] cassandra git commit: 8630: fixed coverity defects
Posted by be...@apache.org.
8630: fixed coverity defects
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/967a2cfe
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/967a2cfe
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/967a2cfe
Branch: refs/heads/trunk
Commit: 967a2cfe179548835d5e8c1640889420ce0d40ce
Parents: 9f0509e
Author: Stefania Alborghetti <st...@datastax.com>
Authored: Mon Sep 7 09:44:46 2015 +0800
Committer: Benedict Elliott Smith <be...@apache.org>
Committed: Thu Sep 10 13:55:06 2015 +0100
----------------------------------------------------------------------
.../cassandra/hints/ChecksummedDataInput.java | 2 +-
.../compress/CompressedRandomAccessReader.java | 30 +++----
.../cassandra/io/util/RandomAccessReader.java | 91 +++++++++-----------
.../cassandra/utils/memory/BufferPool.java | 2 +-
4 files changed, 56 insertions(+), 69 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java b/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
index 543f14e..95ea256 100644
--- a/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
+++ b/src/java/org/apache/cassandra/hints/ChecksummedDataInput.java
@@ -137,7 +137,7 @@ public final class ChecksummedDataInput extends RandomAccessReader.RandomAccessR
private void updateCrc()
{
- if (crcPosition == buffer.position() | crcUpdateDisabled)
+ if (crcPosition == buffer.position() || crcUpdateDisabled)
return;
assert crcPosition >= 0 && crcPosition < buffer.position();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java b/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
index 0242871..7294923 100644
--- a/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
+++ b/src/java/org/apache/cassandra/io/compress/CompressedRandomAccessReader.java
@@ -48,29 +48,13 @@ public class CompressedRandomAccessReader extends RandomAccessReader
protected CompressedRandomAccessReader(Builder builder)
{
- super(builder.initializeBuffers(false));
+ super(builder);
this.metadata = builder.metadata;
this.checksum = metadata.checksumType.newInstance();
- initializeBuffer();
- }
-
- @Override
- protected int getBufferSize(RandomAccessReader.Builder builder)
- {
- // this is the chunk data length, throttling is OK with this
- return builder.bufferSize;
- }
-
- @Override
- protected void initializeBuffer()
- {
- buffer = allocateBuffer(bufferSize);
- buffer.limit(0);
-
if (regions == null)
{
- compressed = allocateBuffer(metadata.compressor().initialCompressedBufferLength(metadata.chunkLength()));
+ compressed = allocateBuffer(metadata.compressor().initialCompressedBufferLength(metadata.chunkLength()), bufferType);
checksumBytes = ByteBuffer.wrap(new byte[4]);
}
}
@@ -110,7 +94,7 @@ public class CompressedRandomAccessReader extends RandomAccessReader
if (compressed.capacity() < chunk.length)
{
BufferPool.put(compressed);
- compressed = allocateBuffer(chunk.length);
+ compressed = allocateBuffer(chunk.length, bufferType);
}
else
{
@@ -278,6 +262,14 @@ public class CompressedRandomAccessReader extends RandomAccessReader
}
@Override
+ protected ByteBuffer createBuffer()
+ {
+ buffer = allocateBuffer(bufferSize, bufferType);
+ buffer.limit(0);
+ return buffer;
+ }
+
+ @Override
public RandomAccessReader build()
{
return new CompressedRandomAccessReader(this);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/io/util/RandomAccessReader.java b/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
index e9b0ee4..43589d0 100644
--- a/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
+++ b/src/java/org/apache/cassandra/io/util/RandomAccessReader.java
@@ -33,9 +33,13 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
// The default buffer size when the client doesn't specify it
public static final int DEFAULT_BUFFER_SIZE = 4096;
- // The maximum buffer size when the limiter is not null, i.e. when throttling
- // is enabled. This is required to avoid aquiring permits that are too large.
- public static final int MAX_THROTTLED_BUFFER_SIZE = 1 << 16; // 64k
+ // The maximum buffer size, we will never buffer more than this size. Further,
+ // when the limiter is not null, i.e. when throttling is enabled, we read exactly
+ // this size, since when throttling the intention is to eventually read everything,
+ // see CASSANDRA-8630
+ // NOTE: this size is chosen both for historical consistency, as a reasonable upper bound,
+ // and because our BufferPool currently has a maximum allocation size of this.
+ public static final int MAX_BUFFER_SIZE = 1 << 16; // 64k
// the IO channel to the file, we do not own a reference to this due to
// performance reasons (CASSANDRA-9379) so it's up to the owner of the RAR to
@@ -57,7 +61,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
protected final int bufferSize;
// the buffer type for buffered readers
- private final BufferType bufferType;
+ protected final BufferType bufferType;
// offset from the beginning of the file
protected long bufferOffset;
@@ -67,42 +71,18 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
protected RandomAccessReader(Builder builder)
{
- super(null);
+ super(builder.createBuffer());
this.channel = builder.channel;
this.regions = builder.regions;
this.limiter = builder.limiter;
this.fileLength = builder.overrideLength <= 0 ? builder.channel.size() : builder.overrideLength;
- this.bufferSize = getBufferSize(builder);
+ this.bufferSize = builder.bufferSize;
this.bufferType = builder.bufferType;
-
- if (builder.bufferSize <= 0)
- throw new IllegalArgumentException("bufferSize must be positive");
-
- if (builder.initializeBuffers)
- initializeBuffer();
- }
-
- protected int getBufferSize(Builder builder)
- {
- if (builder.limiter == null)
- return builder.bufferSize;
-
- // limit to ensure more accurate throttling
- return Math.min(MAX_THROTTLED_BUFFER_SIZE, builder.bufferSize);
- }
-
- protected void initializeBuffer()
- {
- if (regions == null)
- buffer = allocateBuffer(bufferSize);
- else
- buffer = regions.floor(0).buffer.duplicate();
-
- buffer.limit(0);
+ this.buffer = builder.buffer;
}
- protected ByteBuffer allocateBuffer(int size)
+ protected static ByteBuffer allocateBuffer(int size, BufferType bufferType)
{
return BufferPool.get(size, bufferType).order(ByteOrder.BIG_ENDIAN);
}
@@ -396,35 +376,56 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
// The type of the buffer for buffered readers
public BufferType bufferType;
+ // The buffer
+ public ByteBuffer buffer;
+
// The mmap segments for mmap readers
public MmappedRegions regions;
// An optional limiter that will throttle the amount of data we read
public RateLimiter limiter;
- public boolean initializeBuffers;
-
public Builder(ChannelProxy channel)
{
this.channel = channel;
this.overrideLength = -1L;
- this.bufferSize = getBufferSize(DEFAULT_BUFFER_SIZE);
+ this.bufferSize = DEFAULT_BUFFER_SIZE;
this.bufferType = BufferType.OFF_HEAP;
this.regions = null;
this.limiter = null;
- this.initializeBuffers = true;
}
/** The buffer size is typically already page aligned but if that is not the case
- * make sure that it is a multiple of the page size, 4096.
+ * make sure that it is a multiple of the page size, 4096. Also limit it to the maximum
+ * buffer size unless we are throttling, in which case we may as well read the maximum
+ * directly since the intention is to read the full file, see CASSANDRA-8630.
* */
- private static int getBufferSize(int size)
+ private void setBufferSize()
{
- if ((size & ~4095) != size)
+ if (limiter != null)
+ {
+ bufferSize = MAX_BUFFER_SIZE;
+ return;
+ }
+
+ if ((bufferSize & ~4095) != bufferSize)
{ // should already be a page size multiple but if that's not case round it up
- size = (size + 4095) & ~4095;
+ bufferSize = (bufferSize + 4095) & ~4095;
}
- return size;
+
+ bufferSize = Math.min(MAX_BUFFER_SIZE, bufferSize);
+ }
+
+ protected ByteBuffer createBuffer()
+ {
+ setBufferSize();
+
+ buffer = regions == null
+ ? allocateBuffer(bufferSize, bufferType)
+ : regions.floor(0).buffer.duplicate();
+
+ buffer.limit(0);
+ return buffer;
}
public Builder overrideLength(long overrideLength)
@@ -441,7 +442,7 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
if (bufferSize <= 0)
throw new IllegalArgumentException("bufferSize must be positive");
- this.bufferSize = getBufferSize(bufferSize);
+ this.bufferSize = bufferSize;
return this;
}
@@ -463,12 +464,6 @@ public class RandomAccessReader extends RebufferingInputStream implements FileDa
return this;
}
- public Builder initializeBuffers(boolean initializeBuffers)
- {
- this.initializeBuffers = initializeBuffers;
- return this;
- }
-
public RandomAccessReader build()
{
return new RandomAccessReader(this);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/967a2cfe/src/java/org/apache/cassandra/utils/memory/BufferPool.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/utils/memory/BufferPool.java b/src/java/org/apache/cassandra/utils/memory/BufferPool.java
index a64cfb0..f972059 100644
--- a/src/java/org/apache/cassandra/utils/memory/BufferPool.java
+++ b/src/java/org/apache/cassandra/utils/memory/BufferPool.java
@@ -44,7 +44,7 @@ import org.apache.cassandra.utils.concurrent.Ref;
*/
public class BufferPool
{
- /** The size of a page aligned buffer, 64kbit */
+ /** The size of a page aligned buffer, 64KiB */
static final int CHUNK_SIZE = 64 << 10;
@VisibleForTesting