Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IGNITE-21999] Merge partition free-lists into one #3615

Merged
merged 25 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8c24873
IGNITE-21999 Merge partition free-lists into one
Phillippko Apr 16, 2024
4acb234
IGNITE-21999 fixes
Phillippko Apr 16, 2024
b233247
IGNITE-21999 merge DataPageIo implementations
Phillippko Apr 17, 2024
d3fded8
IGNITE-21999 fixes
Phillippko Apr 19, 2024
f37c164
IGNITE-21999 add data type
Phillippko Apr 19, 2024
c52fc74
IGNITE-21999 fixes
Phillippko Apr 22, 2024
3509691
IGNITE-21999 fixes
Phillippko Apr 22, 2024
1f1a487
IGNITE-21999 fixes
Phillippko Apr 22, 2024
c1f2513
IGNITE-21999 make own class for WriteRowHandler
Phillippko Apr 22, 2024
0f04c4a
IGNITE-21999 move inner classes to outer level
Phillippko Apr 22, 2024
7056d48
IGNITE-21999 fix mpd
Phillippko Apr 22, 2024
2411fc4
Merge branch 'main' into ignite-21999
Phillippko Apr 22, 2024
43816f7
IGNITE-21999 javadocs
Phillippko Apr 22, 2024
141f0f1
ignite-21999 review fixes
Phillippko May 1, 2024
fe29873
Merge remote-tracking branch 'origin/ignite-21999' into ignite-21999
Phillippko May 1, 2024
c953720
ignite-21999 remove default methods
Phillippko May 1, 2024
ee24f02
ignite-21999 revert changes
Phillippko May 1, 2024
2c27776
ignite-21999 revert changes
Phillippko May 1, 2024
bea0c40
ignite-21999 revert changes
Phillippko May 1, 2024
86f61cb
ignite-21999 fix
Phillippko May 1, 2024
f46f07b
ignite-21999 fix
Phillippko May 1, 2024
614b73a
ignite-21999 fix
Phillippko May 2, 2024
5a3d049
IGNITE-21999 review fixes
Phillippko May 2, 2024
a8b9a10
IGNITE-21999 fix
Phillippko May 2, 2024
817c898
IGNITE-21999 fix
Phillippko May 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion check-rules/spotbugs-excludes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@
<Match>
<!-- TODO: https://issues.apache.org/jira/browse/IGNITE-21692 -->
<Bug pattern="UPM_UNCALLED_PRIVATE_METHOD"/>
<Class name="org.apache.ignite.internal.pagememory.freelist.AbstractFreeList"/>
<Class name="org.apache.ignite.internal.pagememory.freelist.FreeListImpl"/>
<Method name="initReusedPage"/>
</Match>
<Match>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.ignite.internal.util;

import java.util.Iterator;

/** {@link Iterator} implementation that allows to access the current element multiple times. */
public class CachedIterator<T> implements Iterator<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we shouldn't put it in core, this class is very specific. get contract is weird and hard to explain, so I'd rather hide it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

returned it back to FreeList file

private final Iterator<? extends T> it;

private T next;

public CachedIterator(Iterator<? extends T> it) {
this.it = it;
}

@Override
public boolean hasNext() {
return it.hasNext();
}

@Override
public T next() {
next = it.next();

return next;
}

public T get() {
return next;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.List;
import org.apache.ignite.internal.pagememory.freelist.io.PagesListMetaIo;
import org.apache.ignite.internal.pagememory.freelist.io.PagesListNodeIo;
import org.apache.ignite.internal.pagememory.io.DataPageIo;
import org.apache.ignite.internal.pagememory.io.IoVersions;
import org.apache.ignite.internal.pagememory.io.PageIoModule;
import org.apache.ignite.internal.pagememory.persistence.io.PartitionMetaIo;
Expand All @@ -37,7 +38,8 @@ public Collection<IoVersions<?>> ioVersions() {
return List.of(
PagesListMetaIo.VERSIONS,
PagesListNodeIo.VERSIONS,
PartitionMetaIo.VERSIONS
PartitionMetaIo.VERSIONS,
DataPageIo.VERSIONS
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,22 @@

package org.apache.ignite.internal.pagememory;

import java.nio.ByteBuffer;
import org.apache.ignite.internal.lang.IgniteInternalCheckedException;
import org.apache.ignite.internal.pagememory.io.AbstractDataPageIo;
import org.apache.ignite.internal.pagememory.io.DataPageIo;
import org.apache.ignite.internal.pagememory.io.IoVersions;
import org.jetbrains.annotations.Nullable;

/**
* Simple interface for data, store in some RowStore.
*/
public interface Storable {
/** Number of bytes a data type takes in storage. */
int DATA_TYPE_SIZE_BYTES = 1;

/** Offset of data type from the beginning of the row. */
int DATA_TYPE_OFFSET = 0;

/**
* Sets link for this row.
*
Expand Down Expand Up @@ -59,5 +67,27 @@ public interface Storable {
/**
* Returns I/O for handling this storable.
*/
IoVersions<? extends AbstractDataPageIo<?>> ioVersions();
IoVersions<DataPageIo> ioVersions();
Phillippko marked this conversation as resolved.
Show resolved Hide resolved

/** Returns a byte buffer that contains binary tuple data. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Excuse me, what binary tuple? It's an abstraction, it should not depend on the implementation. There's clearly something wrong here.
What was wrong with writeRowData and writeFragmentData that you could have moved here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed, moved writeRowData and writeFragmentData to storable implementations

@Nullable
ByteBuffer valueBuffer();

/**
* Writes row header to page buffer and moves its pointer to {@link #valueOffset()}.
*
* @param pageBuf page buffer to write header to.
*/
void writeHeader(ByteBuffer pageBuf);

/**
* Writes row header and value to page.
*
* @param pageAddr Page address.
* @param offset Data offset.
*/
void writeToPage(long pageAddr, int offset);

/** Returns value offset from the start of the row. */
int valueOffset();
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

import org.apache.ignite.internal.lang.IgniteInternalCheckedException;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.io.AbstractDataPageIo;
import org.apache.ignite.internal.pagememory.io.DataPageIo;
import org.apache.ignite.internal.pagememory.io.DataPagePayload;
import org.apache.ignite.internal.pagememory.metric.IoStatisticsHolder;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -77,7 +77,7 @@ public <T> void traverse(final long link, PageMemoryTraversal<T> traversal, @Nul
assert pageAddr != 0L : currentLink;

try {
AbstractDataPageIo<?> dataIo = pageMemory.ioRegistry().resolve(pageAddr);
DataPageIo dataIo = pageMemory.ioRegistry().resolve(pageAddr);

int itemId = itemId(currentLink);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

import org.apache.ignite.internal.lang.IgniteInternalCheckedException;
import org.apache.ignite.internal.pagememory.PageMemory;
import org.apache.ignite.internal.pagememory.io.AbstractDataPageIo;
import org.apache.ignite.internal.pagememory.io.DataPageIo;
import org.apache.ignite.internal.pagememory.io.DataPagePayload;
import org.apache.ignite.internal.pagememory.metric.IoStatisticsHolder;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -73,7 +73,7 @@ public T getRowByLink(final long link) throws IgniteInternalCheckedException {
assert pageAddr != 0L : link;

try {
AbstractDataPageIo<?> dataIo = pageMemory.ioRegistry().resolve(pageAddr);
DataPageIo dataIo = pageMemory.ioRegistry().resolve(pageAddr);

int itemId = itemId(link);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public long consumePagePayload(long link, long pageAddr, DataPagePayload payload
}

private long readFullyOrStartReadingFragmented(long pageAddr, DataPagePayload payload) {
assert PageUtils.getByte(pageAddr, payload.offset() + Storable.DATA_TYPE_OFFSET) == dataType();

valueSize = readValueSize(pageAddr, payload);

if (!payload.hasMoreFragments()) {
Expand Down Expand Up @@ -130,6 +132,9 @@ public byte[] result() {
*/
protected abstract int valueOffsetInFirstSlot();

/** Returns type of the data row. */
protected abstract byte dataType();

/**
* Resets the object to make it ready for use.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ protected final void init(long pageId, PageIo init) throws IgniteInternalChecked
* @return Page ID with the incremented rotation ID.
* @see FullPageId
*/
protected final long recyclePage(long pageId, long pageAddr) {
public static long recyclePage(long pageId, long pageAddr) {
long recycled = 0;

if (flag(pageId) == FLAG_DATA) {
Expand All @@ -463,7 +463,7 @@ protected final long recyclePage(long pageId, long pageAddr) {
/**
* Returns a page size without the encryption overhead, in bytes.
*/
protected int pageSize() {
public int pageSize() {
return pageMem.realPageSize(grpId);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,27 @@
import org.apache.ignite.internal.lang.IgniteInternalCheckedException;
import org.apache.ignite.internal.logger.IgniteLogger;
import org.apache.ignite.internal.pagememory.Storable;
import org.apache.ignite.internal.pagememory.metric.IoStatisticsHolder;
import org.apache.ignite.internal.pagememory.util.PageHandler;

/**
* Free list.
*/
public interface FreeList<T extends Storable> {
public interface FreeList {
/**
* Inserts a row.
*
* @param row Row.
* @param statHolder Statistics holder to track IO operations.
* @throws IgniteInternalCheckedException If failed.
*/
void insertDataRow(T row, IoStatisticsHolder statHolder) throws IgniteInternalCheckedException;
void insertDataRow(Storable row) throws IgniteInternalCheckedException;
Phillippko marked this conversation as resolved.
Show resolved Hide resolved

/**
* Inserts rows.
*
* @param rows Rows.
* @param statHolder Statistics holder to track IO operations.
* @throws IgniteInternalCheckedException If failed.
*/
void insertDataRows(Collection<T> rows, IoStatisticsHolder statHolder) throws IgniteInternalCheckedException;
void insertDataRows(Collection<? extends Storable> rows) throws IgniteInternalCheckedException;

/**
* Updates a row by link.
Expand All @@ -54,30 +51,30 @@ public interface FreeList<T extends Storable> {
* @param arg Handler argument.
* @param <S> Argument type.
* @param <R> Result type.
* @param statHolder Statistics holder to track IO operations.
* @return Result.
* @throws IgniteInternalCheckedException If failed.
*/
<S, R> R updateDataRow(
long link,
PageHandler<S, R> pageHnd,
S arg,
IoStatisticsHolder statHolder
S arg
) throws IgniteInternalCheckedException;

/**
* Removes a row by link.
*
* @param link Row link.
* @param statHolder Statistics holder to track IO operations.
* @throws IgniteInternalCheckedException If failed.
*/
void removeDataRowByLink(long link, IoStatisticsHolder statHolder) throws IgniteInternalCheckedException;
void removeDataRowByLink(long link) throws IgniteInternalCheckedException;

/**
* Dump statistics to log.
*
* @param log Logger.
*/
void dumpStatistics(IgniteLogger log);

/** Save metadata without exclusive lock on it. */
void saveMetadata() throws IgniteInternalCheckedException;
}