This patch addresses two cache issues

[sundeepn]

1. 'cache tablename' directive overwrites existing memory tables and can cause memory leaks when the cache command is used on an already cached table. caching new partition also causes a memory leak if the parition already exists.
2. using 'cache' directive on a partitioned table causes table to be corrupt. Only the last cached partition is accessable with the rest being leaked.

[sundeepn]

@rxin - Please review.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[harveyfeng]

These should be checked by Hive's DDL code. If the problem is caused by CACHE <table> handling, then we should add a check in SharkDDLSemanticAnalyzer that throws an exception if the user tries to CACHE a table already stored at MEMORY level

[sundeepn]

This is a problem when we deal with tables that mutate. i.e. folks using cache on partitioned tables or on tables that get data inserted. That creates a need to call cache a second time. I think we need to support repeated calls to cache without an exception.

[harveyfeng]

We could silently ignore repeated cache calls, or return a more descriptive error message saying that the first cache call insures that successive inserts on a MEMORY table will be pinned in memory.

[sundeepn]

That feature does not work. Here is what fails for me:

Create external partitioned table
add partition
cache table
add partition

the second partition is not in memory; and when I query the table, I get partial and incorrect results. i.e. I only get back incorrect results from the first partition and no error to the user. Its more than a exception/messaging issue.

[harveyfeng]

Sorry, I wasn't too clear above. That issue of corrupted partitioned tables
has been addressed in the PR linked in the comments below.
These inline comments about the MemoryMetadataManager workaround address the
concern that reloading entire tables on repeated CACHE calls is redundant
if the first call works correctly (given the fix for partitioned tables).
On Feb 26, 2014 10:48 AM, "sundeepn" notifications@github.com wrote:

In src/main/scala/shark/memstore2/MemoryMetadataManager.scala:

@@ -54,10 +54,16 @@ class MemoryMetadataManager extends LogHelper {
databaseName: String,
tableName: String,
cacheMode: CacheType.CacheType): MemoryTable = {

• val tableKey = MemoryMetadataManager.makeTableKey(databaseName, tableName)
• val newTable = new MemoryTable(databaseName, tableName, cacheMode)
• _tables.put(tableKey, newTable)
• newTable
• val tableKey = MemoryMetadataManager.makeTableKey(databaseName, tableName)
• // Clear out any existing tables with the same key; prevent memory leak
• if (containsTable(databaseName, tableName)) {

•  logInfo("Attempt to create new table when one already exists - " + tableKey)
  

That feature does not work. Here is what fails for me:

Create external partitioned table
add partition
cache table
add partition

the second partition is not in memory; and when I query the table, I get
partial and incorrect results. i.e. I only get back incorrect results from
the first partition and no error to the user. Its more than a
exception/messaging issue.

Reply to this email directly or view it on GitHubhttps://github.com//pull/295/files#r10092827
.

[sundeepn]

i.c. I have not looked at the 0.9 code yet, but in the 0.8.1, the new partition does not get automatically cached without issueing another 'cache tablename' directive. I agree that we should avoid reload of the entire table for each cache directive.

[harveyfeng]

Yeah, the commits will have to be back-ported to 0.8.2.

[harveyfeng]

#2 is actually a bug in SparkLoadTask#loadPartitionedMemoryTable(), I'll submit a fix in a bit

[harveyfeng]

This should address it: #296

[AmplabJenkins]

Merged build finished.

[AmplabJenkins]

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/Shark-Pull-Request-Builder/12151/