timoninmaxim commented on code in PR #11112:
URL: https://github.com/apache/ignite/pull/11112#discussion_r1432695682
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/CreateDumpFutureTask.java:
##########
@@ -517,16 +527,31 @@ public void writeChanged(int cache, long expireTime,
KeyCacheObject key, CacheOb
try {
if (closed) // Partition already saved in dump.
reasonToSkip = "partition already saved";
- else if (isAfterStart(ver))
+ else if (afterStart(ver))
reasonToSkip = "greater version";
- else if (!changed.get(cache).add(key)) // Entry changed
several time during dump.
- reasonToSkip = "changed several times";
- else if (val == null)
- reasonToSkip = "newly created or already removed"; //
Previous value is null. Entry created after dump start, skip.
else {
- write(cache, expireTime, key, val, ver);
-
- changedCnt.increment();
+ try {
+ CacheObjectContext coCtx =
cctx.cacheObjectContext(cache);
+
+ synchronized (serializer) { // Prevent concurrent
access to the dump file.
+ if (writtenByIterator(cache, key, coCtx))
+ reasonToSkip = "written by iterator"; //
Saved by iterator, already. Skip.
+ else if (!changed.get(cache).add(key)) //
Entry changed several time during dump.
+ reasonToSkip = "changed several times";
+ else if (val == null)
+ // Previous value is null. Entry created
after dump start, skip.
+ reasonToSkip = "newly created or already
removed";
+ else {
Review Comment:
Pls, remove curly braces for single line
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/CreateDumpFutureTask.java:
##########
@@ -561,45 +586,60 @@ public boolean writeForIterator(
CacheObject val,
GridCacheVersion ver
) {
- boolean written = true;
+ String reason = null;
if (isAfterStart(ver))
- written = false;
- else if (changed.get(cache).contains(key))
- written = false;
- else
- write(cache, expireTime, key, val, ver);
+ reason = "greater version";
Review Comment:
Docs says it returns false only if entry has been already written. But it's
not true, there are more reasons for skipping entries.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/CreateDumpFutureTask.java:
##########
@@ -610,10 +658,42 @@ private void write(int cache, long expireTime,
KeyCacheObject key, CacheObject v
* @param ver Entry version.
* @return {@code True} if {@code ver} appeared after dump started.
*/
- private boolean isAfterStart(GridCacheVersion ver) {
+ private boolean afterStart(GridCacheVersion ver) {
return (startVer != null && ver.isGreater(startVer)) &&
!isolatedStreamerVer.equals(ver);
}
+ /**
+ * Iterator returned by {@link
IgniteCacheOffheapManager#reservedIterator(int, AffinityTopologyVersion)}
+ * iterates key in ascending order set by {@link
CacheDataTree#compare(BPlusIO, long, int, CacheSearchRow)}.
+ * So if key changed by the user (see {@link #writeChanged(int, long,
KeyCacheObject, CacheObject, GridCacheVersion)})
+ * is greater than last key written by partition iterator then it
hasn't been saved in dump yet and must be written.
+ * Otherwise, key already saved by the iterator and must be skiped.
+ *
+ * @param cache Cache id.
+ * @param key Key to write with {@link #writeChanged(int, long,
KeyCacheObject, CacheObject, GridCacheVersion)}.
+ * @return {@code True} if key writtern by iterator, already. {@code
False} otherwise.
Review Comment:
if the key has already been written by the iterator
--
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]
