Skip to content

[SPARK-17961][SparkR][SQL] Add storageLevel to DataFrame for SparkR #15516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
WeichenXu123 wants to merge 4 commits into from
Closed

[SPARK-17961][SparkR][SQL] Add storageLevel to DataFrame for SparkR #15516

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c11dcce
update
WeichenXu123 Oct 16, 2016
5af4a07
update testcase
WeichenXu123 Oct 21, 2016
cbbeb2d
update
WeichenXu123 Oct 24, 2016
1977591
update.
WeichenXu123 Oct 25, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions R/pkg/NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ exportMethods("arrange",
"selectExpr",
"show",
"showDF",
"storageLevel",
"subset",
"summarize",
"summary",
Expand Down
26 changes: 26 additions & 0 deletions R/pkg/R/DataFrame.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,32 @@ setMethod("unpersist",
x
})

#' StorageLevel
#'
#' Get storagelevel of this SparkDataFrame.
#'
#' @param x the SparkDataFrame to get the storageLevel.
#'
#' @family SparkDataFrame functions
#' @rdname storageLevel-methods
#' @aliases storageLevel,SparkDataFrame-method
#' @name storageLevel
#' @export
#' @examples
#'\dontrun{
#' sparkR.session()
#' path <- "path/to/file.json"
#' df <- read.json(path)
#' persist(df, "MEMORY_AND_DISK")
#' storageLevel(df)
#'}
#' @note storageLevel since 2.1.0
setMethod("storageLevel",
signature(x = "SparkDataFrame"),
function(x) {
storageLevelToString(callJMethod(x@sdf, "storageLevel"))
})

#' Repartition
#'
#' The following options for repartition are possible:
Expand Down
4 changes: 4 additions & 0 deletions R/pkg/R/generics.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,10 @@ setGeneric("selectExpr", function(x, expr, ...) { standardGeneric("selectExpr")
#' @export
setGeneric("showDF", function(x, ...) { standardGeneric("showDF") })

# @rdname storageLevel
# @export
setGeneric("storageLevel", function(x) { standardGeneric("storageLevel") })

#' @rdname subset
#' @export
setGeneric("subset", function(x, ...) { standardGeneric("subset") })
Expand Down
35 changes: 35 additions & 0 deletions R/pkg/R/utils.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,41 @@ getStorageLevel <- function(newLevel = c("DISK_ONLY",
"OFF_HEAP" = callJStatic(storageLevelClass, "OFF_HEAP"))
}

storageLevelToString <- function(levelObj) {
useDisk <- callJMethod(levelObj, "useDisk")
useMemory <- callJMethod(levelObj, "useMemory")
useOffHeap <- callJMethod(levelObj, "useOffHeap")
deserialized <- callJMethod(levelObj, "deserialized")
replication <- callJMethod(levelObj, "replication")
if (!useDisk && !useMemory && !useOffHeap && !deserialized && replication == 1) {

@felixcheung felixcheung Oct 19, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcoding the variations in R could be hard to maintain or easily get out of sync. is there anyway to do this?
Python seems to be able to get the enum name as a string

@WeichenXu123 WeichenXu123 Oct 20, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python has itself StorageLevel class, and the python side code about storageLevel also exists duplicated code problem...
and if we make an r-side StorageLevel class may cause the code more complex and seems won't help solving the duplicated code problem.
What do you think about it ?

@WeichenXu123 WeichenXu123 Oct 20, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and, about the R-side String constant, is there better way to avoid duplicated literal constant in code ? such as "MEMORY_AND_DISK", does we need to define some global vars, such as
MEMORY_AND_DISK_CONSTANT <- "MEMORY_AND_DISK" ?
and where could we put the definition above? if use this way.

@felixcheung felixcheung Oct 21, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Class in R wouldn't help much in this case.
You could have a look up table - check out https://github.com/apache/spark/blob/master/R/pkg/R/types.R and how it is used

"NONE"
} else if (useDisk && !useMemory && !useOffHeap && !deserialized && replication == 1) {
"DISK_ONLY"
} else if (useDisk && !useMemory && !useOffHeap && !deserialized && replication == 2) {
"DISK_ONLY_2"
} else if (!useDisk && useMemory && !useOffHeap && deserialized && replication == 1) {
"MEMORY_ONLY"
} else if (!useDisk && useMemory && !useOffHeap && deserialized && replication == 2) {
"MEMORY_ONLY_2"
} else if (!useDisk && useMemory && !useOffHeap && !deserialized && replication == 1) {
"MEMORY_ONLY_SER"
} else if (!useDisk && useMemory && !useOffHeap && !deserialized && replication == 2) {
"MEMORY_ONLY_SER_2"
} else if (useDisk && useMemory && !useOffHeap && deserialized && replication == 1) {
"MEMORY_AND_DISK"
} else if (useDisk && useMemory && !useOffHeap && deserialized && replication == 2) {
"MEMORY_AND_DISK_2"
} else if (useDisk && useMemory && !useOffHeap && !deserialized && replication == 1) {
"MEMORY_AND_DISK_SER"
} else if (useDisk && useMemory && !useOffHeap && !deserialized && replication == 2) {
"MEMORY_AND_DISK_SER_2"
} else if (useDisk && useMemory && useOffHeap && !deserialized && replication == 1) {
"OFF_HEAP"
} else {
callJMethod(levelObj, "toString")
}
}

# Utility function for functions where an argument needs to be integer but we want to allow
# the user to type (for example) `5` instead of `5L` to avoid a confusing error message.
numToInt <- function(num) {
Expand Down
4 changes: 3 additions & 1 deletion R/pkg/inst/tests/testthat/test_sparkSQL.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ test_that("multiple pipeline transformations result in an RDD with the correct v
expect_false(collectRDD(second)[[3]]$testCol)
})

test_that("cache(), persist(), and unpersist() on a DataFrame", {
test_that("cache(), storageLevel(), persist(), and unpersist() on a DataFrame", {
df <- read.json(jsonPath)
expect_false(df@env$isCached)
cache(df)
Expand All @@ -795,6 +795,8 @@ test_that("cache(), persist(), and unpersist() on a DataFrame", {
persist(df, "MEMORY_AND_DISK")
expect_true(df@env$isCached)

expect_equal(storageLevel(df), "StorageLevel(disk, memory, deserialized, 1 replicas)")

@felixcheung felixcheung Oct 18, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the output of this doesn't say "MEMORY_AND_DISK"? Should we have that in addition to "StorageLevel(disk, memory, deserialized, 1 replicas)"? It might be confusing to set "MEMORY_AND_DISK" and get "StorageLevel(disk, memory, deserialized, 1 replicas)" back?

@WeichenXu123 WeichenXu123 Oct 19, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good suggestion, I'll update the code later. thanks!


unpersist(df)
expect_false(df@env$isCached)

Expand Down