ibessonov commented on code in PR #3615:
URL: https://github.com/apache/ignite-3/pull/3615#discussion_r1587658726
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -154,22 +132,23 @@ public VolatilePageMemory pageMemory() {
}
public ReuseList reuseList() {
- return rowVersionFreeList();
+ return freeList;
}
/**
* Returns version chain free list.
*
* @throws StorageException If the data region did not start.
*/
- public RowVersionFreeList rowVersionFreeList() {
+ public FreeListImpl freeList() {
checkDataRegionStarted();
- return rowVersionFreeList;
+ return freeList;
}
- public IndexColumnsFreeList indexColumnsFreeList() {
- return indexColumnsFreeList;
+ /** Returns pages eviction tracker. */
+ public PageEvictionTracker evictionTracker() {
Review Comment:
Is this necessary?
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/MvPageTypes.java:
##########
@@ -30,9 +30,6 @@ public interface MvPageTypes {
/** Version chain tree leaf page IO type. */
short T_VERSION_CHAIN_LEAF_IO = 11;
- /** Row version data page IO type. */
- short T_ROW_VERSION_DATA_IO = 12;
Review Comment:
What if we re-assign these numbers, to make them more meaningful? This gap
makes no sense now
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/VolatilePageMemoryDataRegion.java:
##########
@@ -154,22 +132,23 @@ public VolatilePageMemory pageMemory() {
}
public ReuseList reuseList() {
- return rowVersionFreeList();
+ return freeList;
}
/**
* Returns version chain free list.
*
* @throws StorageException If the data region did not start.
*/
- public RowVersionFreeList rowVersionFreeList() {
+ public FreeListImpl freeList() {
Review Comment:
I wonder why it returns `impl` and not the interface.
Same applies for places where we declare fields. Your reply would be enough
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/freelist/FreeListImpl.java:
##########
@@ -886,9 +885,18 @@ public long recycledPagesCount() throws
IgniteInternalCheckedException {
}
}
- /** {@inheritDoc} */
+ @Override
+ public void saveMetadata() throws IgniteInternalCheckedException {
+ saveMetadata(statHolder);
+ }
+
+ /** Returns page eviction tracker. */
+ public PageEvictionTracker evictionTracker() {
Review Comment:
Is it still required to be public?
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/Storable.java:
##########
@@ -57,7 +62,49 @@ public interface Storable {
int headerSize();
/**
- * Returns I/O for handling this storable.
+ * Writes the row.
+ *
+ * @param pageAddr Page address.
+ * @param dataOff Data offset.
+ * @param payloadSize Payload size.
+ * @param newRow {@code False} if existing cache entry is updated, in this
case skip key data write.
+ */
+ void writeRowData(
+ long pageAddr,
+ int dataOff,
+ int payloadSize,
+ boolean newRow
+ );
+
+ /**
+ * Writes row data fragment.
+ *
+ * @param pageBuf Byte buffer.
+ * @param rowOff Offset in row data bytes.
+ * @param payloadSize Data length that should be written in a fragment.
+ */
+ void writeFragmentData(
+ ByteBuffer pageBuf,
+ int rowOff,
+ int payloadSize
+ );
+
+ /**
+ * Writes content of the byte buffer into the page.
+ *
+ * @param pageBuffer Direct page buffer.
+ * @param valueBuffer Byte buffer with value bytes.
+ * @param offset Offset within the value buffer.
+ * @param payloadSize Number of bytes to write.
*/
- IoVersions<? extends AbstractDataPageIo<?>> ioVersions();
+ static void putValueBufferIntoPage(ByteBuffer pageBuffer, ByteBuffer
valueBuffer, int offset, int payloadSize) {
+ int oldPosition = valueBuffer.position();
+ int oldLimit = valueBuffer.limit();
+
+ valueBuffer.position(offset);
+ valueBuffer.limit(offset + payloadSize);
+ pageBuffer.put(valueBuffer);
Review Comment:
```suggestion
pageBuffer.put(valueBuffer);
```
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/index/freelist/IndexColumns.java:
##########
@@ -22,17 +22,17 @@
import java.nio.ByteBuffer;
import org.apache.ignite.internal.lang.IgniteInternalCheckedException;
import org.apache.ignite.internal.pagememory.Storable;
-import org.apache.ignite.internal.pagememory.io.AbstractDataPageIo;
-import org.apache.ignite.internal.pagememory.io.IoVersions;
-import
org.apache.ignite.internal.storage.pagememory.index.freelist.io.IndexColumnsDataIo;
+import org.apache.ignite.internal.pagememory.util.PageUtils;
import org.jetbrains.annotations.Nullable;
/**
* Index columns to store in free list.
*/
public class IndexColumns implements Storable {
+ public static final byte DATA_TYPE = 0;
Review Comment:
I would prefer it the other way, data being 0 and index being 1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
