Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/java/org/apache/cassandra/db/marshal/AbstractType.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.apache.cassandra.cql3.CQL3Type;
import org.apache.cassandra.cql3.ColumnSpecification;
import org.apache.cassandra.cql3.Term;
import org.apache.cassandra.db.rows.Cell;
import org.apache.cassandra.exceptions.InvalidColumnTypeException;
import org.apache.cassandra.exceptions.SyntaxException;
import org.apache.cassandra.io.util.DataInputPlus;
Expand Down
4 changes: 2 additions & 2 deletions src/java/org/apache/cassandra/index/sai/IndexContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ public int getIntOption(String name, int defaultValue)
}
catch (NumberFormatException e)
{
logger.error("Failed to parse index configuration " + name + " = " + value + " as integer");
logger.error("Failed to parse index configuration {} = {} as integer", name, value);
return defaultValue;
}
}
Expand Down Expand Up @@ -1003,7 +1003,7 @@ public long indexFileCacheSize()
public IndexFeatureSet indexFeatureSet()
{
IndexFeatureSet.Accumulator accumulator = new IndexFeatureSet.Accumulator(version);
getView().getIndexes().stream().map(SSTableIndex::indexFeatureSet).forEach(set -> accumulator.accumulate(set));
getView().getIndexes().stream().map(SSTableIndex::indexFeatureSet).forEach(accumulator::accumulate);
return accumulator.complete();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ private SSTableContext(SSTableContext copy)
this.primaryKeyMapFactory = copy.primaryKeyMapFactory;
}

@SuppressWarnings("resource")
public static SSTableContext create(SSTableReader sstable, IndexComponents.ForRead perSSTableComponents)
{
var onDiskFormat = perSSTableComponents.onDiskFormat();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public void rangeTombstone(RangeTombstone tombstone)

private void forEach(Consumer<Index.Indexer> action)
{
indexers.forEach(action::accept);
indexers.forEach(action);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this might even save an unnecessary object allocation

}
};
}
Expand Down Expand Up @@ -336,7 +336,7 @@ public void handleNotification(INotification notification, Object sender)

// Avoid validation for index files just written following Memtable flush. ZCS streaming should
// validate index checksum.
boolean validate = notice.fromStreaming() || !notice.memtable().isPresent();
boolean validate = notice.fromStreaming() || notice.memtable().isEmpty();
onSSTableChanged(Collections.emptySet(), notice.added, indices, validate);
}
else if (notification instanceof SSTableListChangedNotification)
Expand Down Expand Up @@ -436,7 +436,7 @@ public int totalIndexBuildsInProgress()
*/
public int totalQueryableIndexCount()
{
return (int) indices.stream().filter(i -> baseCfs.indexManager.isIndexQueryable(i)).count();
return (int) indices.stream().filter(baseCfs.indexManager::isIndexQueryable).count();
}

/**
Expand Down Expand Up @@ -513,7 +513,7 @@ public void unsafeReload()
public void reset()
{
contextManager.clear();
indices.forEach(index -> index.makeIndexNonQueryable());
indices.forEach(StorageAttachedIndex::makeIndexNonQueryable);
onSSTableChanged(baseCfs.getLiveSSTables(), Collections.emptySet(), indices, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,14 @@

import com.google.common.base.Stopwatch;

import org.apache.cassandra.db.DecoratedKey;
import org.apache.cassandra.db.rows.Unfiltered;
import org.apache.cassandra.index.sai.utils.PrimaryKey;

/**
* Writes all SSTable-attached index token and offset structures.
*/
public interface PerSSTableWriter
{
public static final PerSSTableWriter NONE = (key) -> {};
PerSSTableWriter NONE = key -> {};

default void startPartition(long position) throws IOException
{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ public void markComplete() throws IOException
{
addOrGet(completionMarkerComponent()).createEmpty();
sealed = true;
// Until this call, the group is not attached to the parent. This create the link.
// Until this call, the group is not attached to the parent. This creates the link.
updateParentLink(this);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.index.sai.IndexContext;
import org.apache.cassandra.index.sai.SSTableContext;
import org.apache.cassandra.index.sai.SSTableIndex;
import org.apache.cassandra.index.sai.StorageAttachedIndex;
import org.apache.cassandra.index.sai.disk.PerIndexWriter;
import org.apache.cassandra.index.sai.disk.PerSSTableWriter;
Expand Down Expand Up @@ -73,15 +74,15 @@ public interface OnDiskFormat
*
* @return the index feature set
*/
public IndexFeatureSet indexFeatureSet();
IndexFeatureSet indexFeatureSet();

/**
* Returns the {@link PrimaryKey.Factory} for the on-disk format
*
* @param comparator
* @return the primary key factory
*/
public PrimaryKey.Factory newPrimaryKeyFactory(ClusteringComparator comparator);
PrimaryKey.Factory newPrimaryKeyFactory(ClusteringComparator comparator);

/**
* Returns a {@link PrimaryKeyMap.Factory} for the SSTable
Expand All @@ -92,18 +93,18 @@ public interface OnDiskFormat
* @return a {@link PrimaryKeyMap.Factory} for the SSTable
* @throws IOException
*/
public PrimaryKeyMap.Factory newPrimaryKeyMapFactory(IndexComponents.ForRead perSSTableComponents, PrimaryKey.Factory primaryKeyFactory, SSTableReader sstable) throws IOException;
PrimaryKeyMap.Factory newPrimaryKeyMapFactory(IndexComponents.ForRead perSSTableComponents, PrimaryKey.Factory primaryKeyFactory, SSTableReader sstable) throws IOException;

/**
* Create a new {@link SearchableIndex} for an on-disk index. This is held by the {@SSTableIndex}
* Create a new {@link SearchableIndex} for an on-disk index. This is held by the {@link SSTableIndex}
* and shared between queries.
*
* @param sstableContext The {@link SSTableContext} holding the per-SSTable information for the index
* @param perIndexComponents The group of per-index sstable components to use/read for the returned index (which
* also link to the underlying {@link IndexContext} for the index).
* @return the created {@link SearchableIndex}.
*/
public SearchableIndex newSearchableIndex(SSTableContext sstableContext, IndexComponents.ForRead perIndexComponents);
SearchableIndex newSearchableIndex(SSTableContext sstableContext, IndexComponents.ForRead perIndexComponents);

IndexSearcher newIndexSearcher(SSTableContext sstableContext,
IndexContext indexContext,
Expand All @@ -117,7 +118,7 @@ IndexSearcher newIndexSearcher(SSTableContext sstableContext,
* @return The {@link PerSSTableWriter} to write the per-SSTable on-disk components
* @throws IOException
*/
public PerSSTableWriter newPerSSTableWriter(IndexDescriptor indexDescriptor) throws IOException;
PerSSTableWriter newPerSSTableWriter(IndexDescriptor indexDescriptor) throws IOException;

/**
* Create a new writer for the per-index on-disk components of an index. The {@link LifecycleNewTracker}
Expand All @@ -132,10 +133,10 @@ IndexSearcher newIndexSearcher(SSTableContext sstableContext,
* @param keyCount
* @return The {@link PerIndexWriter} that will write the per-index on-disk components
*/
public PerIndexWriter newPerIndexWriter(StorageAttachedIndex index,
IndexDescriptor indexDescriptor,
LifecycleNewTracker tracker,
RowMapping rowMapping, long keyCount);
PerIndexWriter newPerIndexWriter(StorageAttachedIndex index,
IndexDescriptor indexDescriptor,
LifecycleNewTracker tracker,
RowMapping rowMapping, long keyCount);

/**
* Validate the provided on-disk components (that must be for this version).
Expand All @@ -154,7 +155,7 @@ public PerIndexWriter newPerIndexWriter(StorageAttachedIndex index,
*
* @return The set of {@link IndexComponentType} for the per-SSTable index
*/
public Set<IndexComponentType> perSSTableComponentTypes();
Set<IndexComponentType> perSSTableComponentTypes();

/**
* Returns the set of {@link IndexComponentType} for the per-index part of an index.
Expand All @@ -164,12 +165,12 @@ public PerIndexWriter newPerIndexWriter(StorageAttachedIndex index,
* @param indexContext The {@link IndexContext} for the index
* @return The set of {@link IndexComponentType} for the per-index index
*/
default public Set<IndexComponentType> perIndexComponentTypes(IndexContext indexContext)
default Set<IndexComponentType> perIndexComponentTypes(IndexContext indexContext)
{
return perIndexComponentTypes(indexContext.getValidator());
}

public Set<IndexComponentType> perIndexComponentTypes(AbstractType<?> validator);
Set<IndexComponentType> perIndexComponentTypes(AbstractType<?> validator);

/**
* Return the number of open per-SSTable files that can be open during a query.
Expand All @@ -178,7 +179,7 @@ default public Set<IndexComponentType> perIndexComponentTypes(IndexContext index
*
* @return The number of open per-SSTable files
*/
public int openFilesPerSSTable();
int openFilesPerSSTable();

/**
* Return the number of open per-index files that can be open during a query.
Expand All @@ -188,7 +189,7 @@ default public Set<IndexComponentType> perIndexComponentTypes(IndexContext index
* @param indexContext The {@link IndexContext} for the index
* @return The number of open per-index files
*/
public int openFilesPerIndex(IndexContext indexContext);
int openFilesPerIndex(IndexContext indexContext);

/**
* Return the {@link ByteOrder} for the given {@link IndexComponentType} and {@link IndexContext}.
Expand All @@ -197,7 +198,7 @@ default public Set<IndexComponentType> perIndexComponentTypes(IndexContext index
* @param context - The {@link IndexContext} for the index
* @return The {@link ByteOrder} for the file associated with the {@link IndexComponentType}
*/
public ByteOrder byteOrderFor(IndexComponentType component, IndexContext context);
ByteOrder byteOrderFor(IndexComponentType component, IndexContext context);

/**
* Encode the given {@link ByteBuffer} into a {@link ByteComparable} object based on the provided {@link AbstractType}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public PrimaryKey create(DecoratedKey partitionKey, Clustering clustering)
return new PartitionAwarePrimaryKey(partitionKey.getToken(), partitionKey, null);
}

private class PartitionAwarePrimaryKey implements PrimaryKey
private static class PartitionAwarePrimaryKey implements PrimaryKey
{
private final Token token;
private DecoratedKey partitionKey;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.apache.cassandra.index.sai.disk.v1;

import java.io.IOException;
import java.nio.ByteBuffer;

import javax.annotation.concurrent.NotThreadSafe;
import javax.annotation.concurrent.ThreadSafe;
Expand Down Expand Up @@ -106,8 +105,8 @@
@Override
public PrimaryKeyMap newPerSSTablePrimaryKeyMap()
{
final LongArray rowIdToToken = new LongArray.DeferredLongArray(() -> tokenReaderFactory.open());
final LongArray rowIdToOffset = new LongArray.DeferredLongArray(() -> offsetReaderFactory.open());
final LongArray rowIdToToken = new LongArray.DeferredLongArray(tokenReaderFactory::open);
final LongArray rowIdToOffset = new LongArray.DeferredLongArray(offsetReaderFactory::open);

Check failure on line 109 in src/java/org/apache/cassandra/index/sai/disk/v1/PartitionAwarePrimaryKeyMap.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use try-with-resources or close this "DeferredLongArray" in a "finally" clause.

See more on https://sonarcloud.io/project/issues?id=cassandra-stargazer&issues=AZqhSrft5OFznNA8Mvqf&open=AZqhSrft5OFznNA8Mvqf&pullRequest=2131

return new PartitionAwarePrimaryKeyMap(rowIdToToken, rowIdToOffset, partitioner, keyFetcher, primaryKeyFactory, sstableId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@

import org.apache.cassandra.index.sai.disk.format.IndexComponents;
import org.apache.cassandra.index.sai.disk.format.IndexComponentType;
import org.apache.cassandra.index.sai.disk.format.Version;
import org.apache.cassandra.io.util.FileHandle;
import org.apache.cassandra.io.util.FileUtils;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public PrimaryKey.Factory newPrimaryKeyFactory(ClusteringComparator comparator)
}

@Override
public PrimaryKeyMap.Factory newPrimaryKeyMapFactory(IndexComponents.ForRead perSSTableComponents, PrimaryKey.Factory primaryKeyFactory, SSTableReader sstable) throws IOException
public PrimaryKeyMap.Factory newPrimaryKeyMapFactory(IndexComponents.ForRead perSSTableComponents, PrimaryKey.Factory primaryKeyFactory, SSTableReader sstable)
{
return new PartitionAwarePrimaryKeyMap.PartitionAwarePrimaryKeyMapFactory(perSSTableComponents, sstable, primaryKeyFactory);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ PrimaryKey createWithSource(PrimaryKeyMap primaryKeyMap, long sstableRowId, Prim

private class RowAwarePrimaryKey implements PrimaryKey
{
private Token token;
private final Token token;
private DecoratedKey partitionKey;
private Clustering clustering;
private Supplier<PrimaryKey> primaryKeySupplier;
Expand Down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove perSSTableComponents too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might lead to memory leaks. I will re-test it, since previously I had more than one change, so the memory leak might related to similar but different change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove perSSTableComponents too?

I tested removing it from unnecessary storing as a class field and several tests failed with memory leaks. This looks to me a tech debt and potential source for memory leak bugs in future, and fixing it would require refactoring.
@michaeljmarshall what do you think about it? Is it important to create an issue and fix in future?

Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,10 @@ public RowAwarePrimaryKeyMapFactory(IndexComponents.ForRead perSSTableComponents
@Override
public PrimaryKeyMap newPerSSTablePrimaryKeyMap()
{
final LongArray rowIdToToken = new LongArray.DeferredLongArray(() -> tokenReaderFactory.open());
final LongArray rowIdToToken = new LongArray.DeferredLongArray(tokenReaderFactory::open);
try
{
return new RowAwarePrimaryKeyMap(rowIdToToken,
sortedTermsReader,
sortedTermsReader.openCursor(),
partitioner,
primaryKeyFactory,
Expand Down Expand Up @@ -149,7 +148,6 @@ public void close() throws IOException
}

private final LongArray rowIdToToken;
private final SortedTermsReader sortedTermsReader;
private final SortedTermsReader.Cursor cursor;
private final IPartitioner partitioner;
private final RowAwarePrimaryKeyFactory primaryKeyFactory;
Expand All @@ -158,7 +156,6 @@ public void close() throws IOException
private final boolean hasStaticColumns;

private RowAwarePrimaryKeyMap(LongArray rowIdToToken,
SortedTermsReader sortedTermsReader,
SortedTermsReader.Cursor cursor,
IPartitioner partitioner,
RowAwarePrimaryKeyFactory primaryKeyFactory,
Expand All @@ -167,7 +164,6 @@ private RowAwarePrimaryKeyMap(LongArray rowIdToToken,
boolean hasStaticColumns)
{
this.rowIdToToken = rowIdToToken;
this.sortedTermsReader = sortedTermsReader;
this.cursor = cursor;
this.partitioner = partitioner;
this.primaryKeyFactory = primaryKeyFactory;
Expand Down Expand Up @@ -235,10 +231,10 @@ public long exactRowIdOrInvertedCeiling(PrimaryKey key)
if (clusteringComparator.size() == 0)
return skinnyExactRowIdOrInvertedCeiling(key);

long pointId = cursor.getExactPointId(v -> key.asComparableBytes(v));
long pointId = cursor.getExactPointId(key::asComparableBytes);
if (pointId >= 0)
return pointId;
long ceiling = cursor.ceiling(v -> key.asComparableBytesMinPrefix(v));
long ceiling = cursor.ceiling(key::asComparableBytesMinPrefix);
// Use min value since -(Long.MIN_VALUE) - 1 == Long.MAX_VALUE.
return ceiling < 0 ? Long.MIN_VALUE : -ceiling - 1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public class Cursor implements AutoCloseable
// The point id the cursor currently points to. -1 means before the first item.
private long pointId = -1;

private TrieTermsDictionaryReader reader;
private final TrieTermsDictionaryReader reader;

Cursor(FileHandle termsData, LongArray.Factory blockOffsetsFactory) throws IOException
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ public class SortedTermsWriter implements Closeable
static final int TERMS_DICT_BLOCK_SIZE = 1 << TERMS_DICT_BLOCK_SHIFT;
static final int TERMS_DICT_BLOCK_MASK = TERMS_DICT_BLOCK_SIZE - 1;

static final int DIRECT_MONOTONIC_BLOCK_SHIFT = 16;

private final IncrementalTrieWriter<Long> trieWriter;
private final IndexOutputWriter trieOutput;
private final IndexOutput termsOutput;
Expand Down
4 changes: 2 additions & 2 deletions src/java/org/apache/cassandra/io/sstable/SSTable.java
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ public long bytesOnDisk()
@Override
public String toString()
{
return getClass().getSimpleName() + "(" +
return getClass().getSimpleName() + '(' +
"path='" + getFilename() + '\'' +
')';
}
Expand Down Expand Up @@ -403,7 +403,7 @@ private static void rewriteTOC(Descriptor descriptor, Collection<Component> comp
*/
public static void writeTOC(File tocFile, Collection<Component> components, File.WriteMode writeMode)
{
FileOutputStreamPlus fos = null;
FileOutputStreamPlus fos;
try (PrintWriter w = new PrintWriter((fos = tocFile.newOutputStream(writeMode))))
{
for (Component component : components)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public abstract class AbstractTrieMemoryIndexBenchmark
private static final String INTEGER_INDEX = "integer_index";
private static final int RANDOM_STRING_SIZE = 64 * 1024 * 1024;

private char[] randomChars = new char[RANDOM_STRING_SIZE];
private final char[] randomChars = new char[RANDOM_STRING_SIZE];

protected int randomSeed;

Expand Down
Loading