timoninmaxim commented on code in PR #11309:
URL: https://github.com/apache/ignite/pull/11309#discussion_r1559304156
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/Dump.java:
##########
@@ -146,8 +146,8 @@ public Dump(
this.consistentId = consistentId == null ? null :
U.maskForFileName(consistentId);
this.metadata = metadata(dumpDir, this.consistentId);
this.keepBinary = keepBinary;
- this.cctx = standaloneKernalContext(dumpDir, log);
Review Comment:
Wy do you need this change?
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/IgniteCacheDumpSelf2Test.java:
##########
@@ -170,6 +174,85 @@ public class IgniteCacheDumpSelf2Test extends
GridCommonAbstractTest {
cleanPersistenceDir();
}
+ /** */
+ @Test
+ public void testDumpRawData() throws Exception {
+ IgniteEx ign = startGrids(3);
+
+ Ignite cli = startClientGrid(G.allGrids().size());
+
+ cli.createCache(defaultCacheConfiguration());
+
+ for (int i = 0; i < KEYS_CNT; ++i)
+ cli.cache(DEFAULT_CACHE_NAME).put(i, USER_FACTORY.apply(i));
+
+ cli.snapshot().createDump(DMP_NAME, null).get();
+
+ AtomicBoolean keepRaw = new AtomicBoolean();
+ AtomicBoolean keepBinary = new AtomicBoolean();
+
+ DumpConsumer cnsmr = new DumpConsumer() {
+ @Override public void start() {
+ // No-op.
+ }
+
+ @Override public void onMappings(Iterator<TypeMapping> mappings) {
+ // No-op.
+ }
+
+ @Override public void onTypes(Iterator<BinaryType> types) {
+ // No-op.
+ }
+
+ @Override public void onCacheConfigs(Iterator<StoredCacheData>
caches) {
+ // No-op.
+ }
+
+ @Override public void onPartition(int grp, int part,
Iterator<DumpEntry> data) {
+ data.forEachRemaining(de -> {
+ if (keepRaw.get()) {
+ assertTrue(de.key() instanceof KeyCacheObject);
+ assertTrue(de.value() instanceof CacheObject);
Review Comment:
`de.value()` is a `BinaryObjectImpl` in both cases: `keepRaw=true` and
`keepBinary=true`. Looks like we need to test value types that differs from
user defined classes to show difference for value types
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/AbstractSnapshotSelfTest.java:
##########
@@ -1079,7 +1079,7 @@ public Account(int id, int balance) {
}
/** */
- protected static class BlockingExecutor implements Executor {
+ public static class BlockingExecutor implements Executor {
Review Comment:
It's a bad practice make the internal static classes public. Let's revert
these changes, maybe it's better to inherit the test and invoke
`doSnapshotCancellationTest` method?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/Dump.java:
##########
@@ -398,7 +403,8 @@ private File dumpGroupDirectory(String node, int grpId) {
* Closeable dump iterator.
*/
public interface DumpedPartitionIterator extends Iterator<DumpEntry>,
AutoCloseable {
- // No-op.
+ /** Path to current node's data. */
+ String nodeDataPath();
Review Comment:
Why do you need this change?
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/dump/IgniteCacheDumpSelf2Test.java:
##########
@@ -170,6 +174,85 @@ public class IgniteCacheDumpSelf2Test extends
GridCommonAbstractTest {
cleanPersistenceDir();
}
+ /** */
+ @Test
+ public void testDumpRawData() throws Exception {
+ IgniteEx ign = startGrids(3);
+
+ Ignite cli = startClientGrid(G.allGrids().size());
+
+ cli.createCache(defaultCacheConfiguration());
+
+ for (int i = 0; i < KEYS_CNT; ++i)
+ cli.cache(DEFAULT_CACHE_NAME).put(i, USER_FACTORY.apply(i));
+
+ cli.snapshot().createDump(DMP_NAME, null).get();
+
+ AtomicBoolean keepRaw = new AtomicBoolean();
+ AtomicBoolean keepBinary = new AtomicBoolean();
+
+ DumpConsumer cnsmr = new DumpConsumer() {
+ @Override public void start() {
+ // No-op.
+ }
+
+ @Override public void onMappings(Iterator<TypeMapping> mappings) {
+ // No-op.
+ }
+
+ @Override public void onTypes(Iterator<BinaryType> types) {
+ // No-op.
+ }
+
+ @Override public void onCacheConfigs(Iterator<StoredCacheData>
caches) {
+ // No-op.
+ }
+
+ @Override public void onPartition(int grp, int part,
Iterator<DumpEntry> data) {
+ data.forEachRemaining(de -> {
+ if (keepRaw.get()) {
+ assertTrue(de.key() instanceof KeyCacheObject);
+ assertTrue(de.value() instanceof CacheObject);
+ }
+ else {
+ assertTrue(de.key() instanceof Integer);
+
+ if (keepBinary.get())
+ assertTrue(de.value() instanceof BinaryObject);
+ else
+ assertTrue(de.value() instanceof User);
+ }
+ });
+ }
+
+ @Override public void stop() {
+ // No-op.
+ }
+ };
+
+ for (boolean[] binaryRaw : Arrays.asList(new boolean[] {false, true},
new boolean[] {false, false},
Review Comment:
Looks like using 2 loops is more readable?
--
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]
