[SPARK-11244][SPARKR] sparkR.stop() should remove SQLContext

[saurfang]

SparkR should remove .sparkRSQLsc and .sparkRHivesc when sparkR.stop() is called. Otherwise even when SparkContext is reinitialized, sparkRSQL.init returns the stale copy of the object and complains:

sc <- sparkR.init("local")
sqlContext <- sparkRSQL.init(sc)
sparkR.stop()
sc <- sparkR.init("local")
sqlContext <- sparkRSQL.init(sc)
sqlContext

producing

Error in callJMethod(x, "getClass") : 
  Invalid jobj 1. If SparkR was restarted, Spark operations need to be re-executed.

I have added the check and removal only when SparkContext itself is initialized. I have also added corresponding test for this fix. Let me know if you want me to move the test to SQL test suite instead.

p.s. I tried lint-r but ended up a lots of errors on existing code.

[shivaram]

Thanks @saurfang -- Test is fine in the current location. Just curious what are the lint-r errors you saw ?

[saurfang]

Most of them are "style: Commented code should be removed."

For example:

R/RDD.R:260:3: style: Commented code should be removed.
# unpersist(rdd) # rdd@@env$isCached == FALSE
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
R/RDD.R:283:3: style: Commented code should be removed.
# sc <- sparkR.init()
  ^~~~~~~~~~~~~~~~~~~
R/RDD.R:284:3: style: Commented code should be removed.
# setCheckpointDir(sc, "checkpoint")
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Looks like to be a new feature introduced at: r-lib/lintr#83
Also shouldn't # be #' for roxygen doc?

[shivaram]

Ah I see. Hmm we use # instead of #' for a bunch of cases where we dont want roxygen to parse it as the functions are not part of the public API. Not sure if there is a better way to do this.

cc @yu-iskw

[SparkQA]

Test build #44086 has finished for PR 9205 at commit 7f51549.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[saurfang]

I see. Thanks for the information. I thought the convention in R package is to expose as much documentation as possible but don't export functions that is unstable or developer only. If users choose to use them, they acknowledge the risk by using :::. The linter complains only for commented R code rather than any comments. I feel if we already spent the time to write examples, both users and package developers should and would benefit from these.

[sun-rui]

is it possible for lin-r to skip # comment with code outside a function body?

[saurfang]

I still advocate making developer API documentation public. However I think one workaround is to make them roxygen doc again with #' but add #' @rdname .ignore for each function. Roxygen will skip these with message Skipping invalid path: .ignore.Rd

[shivaram]

@saurfang Can you open a new JIRA for these style failures ? We can discuss the points there.

LGTM. Merging this in master and branch-1.5

[shivaram]

cc @olarayej

[saurfang]

Thanks @shivaram! Opened https://issues.apache.org/jira/browse/SPARK-11263 for further discussion.