Phillippko commented on code in PR #4379:
URL: https://github.com/apache/ignite-3/pull/4379#discussion_r1756565035
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointDirtyPages.java:
##########
@@ -44,23 +42,24 @@ class CheckpointDirtyPages {
/** Empty checkpoint dirty pages. */
static final CheckpointDirtyPages EMPTY = new
CheckpointDirtyPages(List.of());
- /** Dirty pages of data regions, with sorted page IDs by {@link
#DIRTY_PAGE_COMPARATOR} and unsorted partition IDs. */
- private final List<DataRegionDirtyPages<FullPageId[]>> dirtyPages;
+ /** Dirty pages and partitions of data regions, with sorted dirty page IDs
by {@link #DIRTY_PAGE_COMPARATOR}. */
+ private final List<DirtyPagesAndPartitions> dirtyPagesAndPartitions;
/** Total number of dirty page IDs. */
private final int dirtyPagesCount;
/**
* Constructor.
*
- * @param dirtyPages Dirty pages of data regions, with sorted page IDs by
{@link #DIRTY_PAGE_COMPARATOR} and unsorted partition IDs.
+ * @param dirtyPagesAndPartitions Dirty pages and partitions of data
regions, with sorted dirty page IDs by
Review Comment:
Let's add that it must support random access
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointDirtyPages.java:
##########
@@ -89,8 +89,8 @@ public IgniteConcurrentMultiPairQueue<PersistentPageMemory,
FullPageId> toDirtyP
* @param partId Partition ID.
*/
public @Nullable CheckpointDirtyPagesView
getPartitionView(PersistentPageMemory pageMemory, int grpId, int partId) {
Review Comment:
I see a usage in "writePageToDeltaFilePageStore" that is not checking if
returned partition view is null
##########
modules/page-memory/src/main/java/org/apache/ignite/internal/pagememory/persistence/checkpoint/CheckpointPagesWriter.java:
##########
@@ -176,90 +166,85 @@ public void run() {
}
}
- /**
- * Writes dirty pages.
- *
- * @param writePageIds Queue of dirty page IDs to write.
- * @return pagesToRetry Queue dirty page IDs which should be retried.
- */
- private IgniteConcurrentMultiPairQueue<PersistentPageMemory, FullPageId>
writePages(
- IgniteConcurrentMultiPairQueue<PersistentPageMemory, FullPageId>
writePageIds
- ) throws IgniteInternalCheckedException {
- Map<PersistentPageMemory, List<FullPageId>> pageIdsToRetry = new
HashMap<>();
-
- Map<PersistentPageMemory, PageStoreWriter> pageStoreWriters = new
HashMap<>();
+ private void writeDirtyPages(PersistentPageMemory pageMemory,
GroupPartitionId partitionId) throws IgniteInternalCheckedException {
+ CheckpointDirtyPagesView checkpointDirtyPagesView =
checkpointDirtyPagesView(pageMemory, partitionId);
- // Page store writers for checkpoint buffer pages are located in a
separate map, because they would not add elements to a
- // "pageIdsToRetry" map. Instead, they would ignore unsuccessful write
lock acquisitions. It's implemented this way in order to
- // avoid duplicates in "pageIdsToRetry", there must only be a single
source of pages to achieve that.
- Map<PersistentPageMemory, PageStoreWriter> cpBufferPageStoreWriters =
new HashMap<>();
+ checkpointProgress.onStartPartitionProcessing(partitionId);
- ByteBuffer tmpWriteBuf = threadBuf.get();
+ try {
+ ByteBuffer tmpWriteBuf = threadBuf.get();
- Result<PersistentPageMemory, FullPageId> queueResult = new Result<>();
+ if (shouldWriteMetaPage(partitionId)) {
+ writePartitionMeta(pageMemory, partitionId,
tmpWriteBuf.rewind());
+ }
- GroupPartitionId partitionId = null;
+ var pageIdsToRetry = new ArrayList<FullPageId>();
- try {
- // TODO https://issues.apache.org/jira/browse/IGNITE-23115 Try to
write file per thread.
- while (!shutdownNow.getAsBoolean() &&
writePageIds.next(queueResult)) {
+ for (int i = 0; i < checkpointDirtyPagesView.size() &&
!shutdownNow.getAsBoolean(); i++) {
updateHeartbeat.run();
- FullPageId fullId = queueResult.getValue();
+ FullPageId pageId = checkpointDirtyPagesView.get(i);
- PersistentPageMemory pageMemory = queueResult.getKey();
+ if (pageId.pageIdx() == 0) {
+ // Skip meta-pages, they are written by
"writePartitionMeta".
+ continue;
+ }
- if (hasPartitionChanged(partitionId, fullId)) {
- GroupPartitionId newPartitionId = toPartitionId(fullId);
+ writeDirtyPage(pageMemory, pageId, tmpWriteBuf,
pageIdsToRetry);
+ }
- // Starting for the new partition.
-
checkpointProgress.onStartPartitionProcessing(newPartitionId);
+ writeRetryDirtyPages(pageMemory, pageIdsToRetry, tmpWriteBuf);
+ } finally {
+ checkpointProgress.onFinishPartitionProcessing(partitionId);
+ }
+ }
- if (partitionId != null) {
- // Finishing for the previous partition.
- // TODO
https://issues.apache.org/jira/browse/IGNITE-23105 Reimplement partition
destruction awaiting.
Review Comment:
TODO is lost
--
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]
