[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame) URL: https://github.com/apache/ignite/pull/6907#discussion_r330017134 ## File path: modules/spark/src/test/java/org/apache/ignite/spark/JavaEmbeddedIgniteRDDSelfTest.java ## @@ -335,4 +412,25 @@ private static IgniteConfiguration getConfiguration(String igniteInstanceName, b return t._2(); } } + +/** + * Review comment: Nit: We should write a javadoc here or replace it with the 1 line stub `/** */` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r330016627
##
File path:
modules/spark/src/test/java/org/apache/ignite/spark/JavaEmbeddedIgniteRDDSelfTest.java
##
@@ -150,6 +169,59 @@ public void testStoreDataToIgnite() throws Exception {
}
}
+/**
+ * @throws Exception If failed.
+ */
+@Test
+public void testStoreDataToIgniteWithEnabledOptionSkipStore() throws
Exception {
+storeMap = new ConcurrentHashMap<>();
+store = new TestStore();
+
+JavaSparkContext sc = createContext();
+
+JavaIgniteContext ic = null;
+
+try {
+ic = new JavaIgniteContext<>(sc, new IgniteConfigProvider(),
false);
+
+Ignite ignite = ic.ignite();
+
+IgniteCache cache =
ignite.cache(PARTITIONED_CACHE_NAME);
+
+// 1000 entries in store; 1000 entries in cache
+for (int i = 0; i < 1000; i++)
+storeMap.put(i, i);
+
+// 1000 entries in store and cache; 1000 another entries in cache
writing to store
+ic.fromCache(PARTITIONED_CACHE_NAME)
+.savePairs(sc.parallelize(F.range(1000, 2000),
GRID_CNT).mapToPair(SIMPLE_FUNCTION), true, false);
+
+// check 2000 entries in store
+for (int i = 0; i < 2000; i++)
+assertEquals(i, storeMap.get(i));
+
+// add 1000 entries to cache but skip store
Review comment:
Nit: The sentence should start with the capital letter and ends with the dot.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r330016504
##
File path:
modules/spark/src/test/java/org/apache/ignite/spark/JavaEmbeddedIgniteRDDSelfTest.java
##
@@ -150,6 +169,59 @@ public void testStoreDataToIgnite() throws Exception {
}
}
+/**
+ * @throws Exception If failed.
+ */
+@Test
+public void testStoreDataToIgniteWithEnabledOptionSkipStore() throws
Exception {
+storeMap = new ConcurrentHashMap<>();
+store = new TestStore();
+
+JavaSparkContext sc = createContext();
+
+JavaIgniteContext ic = null;
+
+try {
+ic = new JavaIgniteContext<>(sc, new IgniteConfigProvider(),
false);
+
+Ignite ignite = ic.ignite();
+
+IgniteCache cache =
ignite.cache(PARTITIONED_CACHE_NAME);
+
+// 1000 entries in store; 1000 entries in cache
+for (int i = 0; i < 1000; i++)
+storeMap.put(i, i);
+
+// 1000 entries in store and cache; 1000 another entries in cache
writing to store
Review comment:
Nit: The sentence should start with the capital letter and ends with the dot.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r330016470
##
File path:
modules/spark/src/test/java/org/apache/ignite/spark/JavaEmbeddedIgniteRDDSelfTest.java
##
@@ -150,6 +169,59 @@ public void testStoreDataToIgnite() throws Exception {
}
}
+/**
+ * @throws Exception If failed.
+ */
+@Test
+public void testStoreDataToIgniteWithEnabledOptionSkipStore() throws
Exception {
+storeMap = new ConcurrentHashMap<>();
+store = new TestStore();
+
+JavaSparkContext sc = createContext();
+
+JavaIgniteContext ic = null;
+
+try {
+ic = new JavaIgniteContext<>(sc, new IgniteConfigProvider(),
false);
+
+Ignite ignite = ic.ignite();
+
+IgniteCache cache =
ignite.cache(PARTITIONED_CACHE_NAME);
+
+// 1000 entries in store; 1000 entries in cache
+for (int i = 0; i < 1000; i++)
+storeMap.put(i, i);
+
+// 1000 entries in store and cache; 1000 another entries in cache
writing to store
+ic.fromCache(PARTITIONED_CACHE_NAME)
+.savePairs(sc.parallelize(F.range(1000, 2000),
GRID_CNT).mapToPair(SIMPLE_FUNCTION), true, false);
+
+// check 2000 entries in store
Review comment:
Nit: The sentence should start with the capital letter and ends with the dot.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r330016533
##
File path:
modules/spark/src/test/java/org/apache/ignite/spark/JavaEmbeddedIgniteRDDSelfTest.java
##
@@ -150,6 +169,59 @@ public void testStoreDataToIgnite() throws Exception {
}
}
+/**
+ * @throws Exception If failed.
+ */
+@Test
+public void testStoreDataToIgniteWithEnabledOptionSkipStore() throws
Exception {
+storeMap = new ConcurrentHashMap<>();
+store = new TestStore();
+
+JavaSparkContext sc = createContext();
+
+JavaIgniteContext ic = null;
+
+try {
+ic = new JavaIgniteContext<>(sc, new IgniteConfigProvider(),
false);
+
+Ignite ignite = ic.ignite();
+
+IgniteCache cache =
ignite.cache(PARTITIONED_CACHE_NAME);
+
+// 1000 entries in store; 1000 entries in cache
Review comment:
Nit: The sentence should start with the capital letter and ends with the dot.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r328527506
##
File path:
modules/spark/src/main/scala/org/apache/ignite/spark/JavaIgniteRDD.scala
##
@@ -82,20 +82,21 @@ class JavaIgniteRDD[K, V](override val rdd: IgniteRDD[K,
V])
def saveValues[T](jrdd: JavaRDD[T], f: (T, IgniteContext) ⇒ V) =
rdd.saveValues(JavaRDD.toRDD(jrdd), f)
-def savePairs(jrdd: JavaPairRDD[K, V], overwrite: Boolean) = {
+def savePairs(jrdd: JavaPairRDD[K, V], overwrite: Boolean, skipStore:
Boolean) = {
val rrdd: RDD[(K, V)] = JavaPairRDD.toRDD(jrdd)
-rdd.savePairs(rrdd, overwrite)
+rdd.savePairs(rrdd, overwrite, skipStore)
}
-def savePairs(jrdd: JavaPairRDD[K, V]) : Unit = savePairs(jrdd, overwrite
= false)
+def savePairs(jrdd: JavaPairRDD[K, V]): Unit = savePairs(jrdd, overwrite =
false, skipStore = false)
-def savePairs[T](jrdd: JavaRDD[T], f: (T, IgniteContext) ⇒ (K, V),
overwrite: Boolean = false) = {
-rdd.savePairs(JavaRDD.toRDD(jrdd), f, overwrite)
+def savePairs[T](jrdd: JavaRDD[T], f: (T, IgniteContext) ⇒ (K, V),
overwrite: Boolean = false,
Review comment:
Seems, these two lines can be combined in one.
I double-checked and seems it not.
Sorry, just skip this comment.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r328311330
##
File path:
modules/spark/src/main/scala/org/apache/ignite/spark/JavaIgniteRDD.scala
##
@@ -82,20 +82,21 @@ class JavaIgniteRDD[K, V](override val rdd: IgniteRDD[K,
V])
def saveValues[T](jrdd: JavaRDD[T], f: (T, IgniteContext) ⇒ V) =
rdd.saveValues(JavaRDD.toRDD(jrdd), f)
-def savePairs(jrdd: JavaPairRDD[K, V], overwrite: Boolean) = {
+def savePairs(jrdd: JavaPairRDD[K, V], overwrite: Boolean, skipStore:
Boolean) = {
val rrdd: RDD[(K, V)] = JavaPairRDD.toRDD(jrdd)
-rdd.savePairs(rrdd, overwrite)
+rdd.savePairs(rrdd, overwrite, skipStore)
}
-def savePairs(jrdd: JavaPairRDD[K, V]) : Unit = savePairs(jrdd, overwrite
= false)
+def savePairs(jrdd: JavaPairRDD[K, V]): Unit = savePairs(jrdd, overwrite =
false, skipStore = false)
-def savePairs[T](jrdd: JavaRDD[T], f: (T, IgniteContext) ⇒ (K, V),
overwrite: Boolean = false) = {
-rdd.savePairs(JavaRDD.toRDD(jrdd), f, overwrite)
+def savePairs[T](jrdd: JavaRDD[T], f: (T, IgniteContext) ⇒ (K, V),
overwrite: Boolean = false,
Review comment:
Redundant NL
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame) URL: https://github.com/apache/ignite/pull/6907#discussion_r328311065 ## File path: modules/spark/src/main/scala/org/apache/ignite/spark/IgniteRDD.scala ## @@ -254,12 +255,13 @@ class IgniteRDD[K, V] ( /** * Saves values from the given RDD into Ignite. * - * @param rdd RDD instance to save values from. - * @param f Transformation function. - * @param overwrite Boolean flag indicating whether the call on this method should overwrite existing - * values in Ignite cache. + * @param rddRDD instance to save values from. Review comment: Why thins reformatting? Please, revert unnecessary format changes. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r328311012
##
File path: modules/spark/src/main/scala/org/apache/ignite/spark/IgniteRDD.scala
##
@@ -223,13 +222,14 @@ class IgniteRDD[K, V] (
}
/**
- * Saves values from the given key-value RDD into Ignite.
- *
- * @param rdd RDD instance to save values from.
- * @param overwrite Boolean flag indicating whether the call on this
method should overwrite existing
- * values in Ignite cache.
- */
-def savePairs(rdd: RDD[(K, V)], overwrite: Boolean = false) = {
+ * Saves values from the given key-value RDD into Ignite.
Review comment:
Why thins reformatting?
Please, revert unnecessary format changes.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r328310665
##
File path: modules/spark/src/main/scala/org/apache/ignite/spark/IgniteRDD.scala
##
@@ -223,13 +222,14 @@ class IgniteRDD[K, V] (
}
/**
- * Saves values from the given key-value RDD into Ignite.
- *
- * @param rdd RDD instance to save values from.
- * @param overwrite Boolean flag indicating whether the call on this
method should overwrite existing
- * values in Ignite cache.
- */
-def savePairs(rdd: RDD[(K, V)], overwrite: Boolean = false) = {
+ * Saves values from the given key-value RDD into Ignite.
Review comment:
Seems you added space for existing comment.
Please, remove it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark integration should support skipStore option for internal dataStreamer (IgniteRdd and Ignite DataFrame)
nizhikov commented on a change in pull request #6907: IGNITE-11723: IgniteSpark
integration should support skipStore option for internal dataStreamer
(IgniteRdd and Ignite DataFrame)
URL: https://github.com/apache/ignite/pull/6907#discussion_r328310665
##
File path: modules/spark/src/main/scala/org/apache/ignite/spark/IgniteRDD.scala
##
@@ -223,13 +222,14 @@ class IgniteRDD[K, V] (
}
/**
- * Saves values from the given key-value RDD into Ignite.
- *
- * @param rdd RDD instance to save values from.
- * @param overwrite Boolean flag indicating whether the call on this
method should overwrite existing
- * values in Ignite cache.
- */
-def savePairs(rdd: RDD[(K, V)], overwrite: Boolean = false) = {
+ * Saves values from the given key-value RDD into Ignite.
Review comment:
Seems you added space for existing comment.
Please, remove it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
