[ 
https://issues.apache.org/jira/browse/CASSANDRA-2392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13190792#comment-13190792
 ] 

Pavel Yaskevich commented on CASSANDRA-2392:
--------------------------------------------

{quote}
Well thats for the following...
if (!recreatebloom && !cacheLoading && loadSummaries)
return;

We still need to complete the indexes and builders even though you dont go 
through all the code in try.
{quote}

Then I can suggest you to skip the primary_index loop phrase instead of return, 
because using finally that way is not a good practice nor style.

bq. Not sure if it can have different semantics, but i will remove the refactor 
not a big deal.

Please do.

bq. I dont think in either case we leak descriptors it is in the finally block 
already and all we are planning to save is a variable assignment and i will do 
that not a big deal.

I don't follow here, what I mean is to change {load, save}Summaries method's 
try blocks to look like code posted below, because in your version you close 
stream only if IOException is thrown:

{noformat}
+        try
+        {
+            {iStream = new DataInputStream(new 
FileInputStream(inMemoryDataFile));
+            reader.indexSummary = IndexSummary.serializer.deserialize(iStream, 
reader.descriptor);
+            ibuilder.deserializeBounds(iStream);
+            dbuilder.deserializeBounds(iStream);
+        }
+        catch (IOException e)
+        {
+            // corrupted hence delete it and let it load it now.
+            if (inMemoryDataFile.exists())
+                inMemoryDataFile.delete();
+            return false;
+        }
+        finally
+        {
+            FileUtils.closeQuietly(iStream);
+        }
{noformat}

and

{noformat}
+        try
+        {
+            oStream = new DataOutputStream(new FileOutputStream(summaryFile));
+            IndexSummary.serializer.serialize(reader.indexSummary, oStream);
+            ibuilder.serializeBounds(oStream);
+            dbuilder.serializeBounds(oStream);
+        }
+        catch (IOException e)
+        {
+            // corrupted hence delete it and let it load it now.
+            if (summaryFile.exists())
+                summaryFile.delete();
+        }
+        finally
+        {
+            FileUtils.closeQuietly(oStream);
+        }
{noformat}

To make sure that we close stream every time and not only when IOException is 
thrown,file descriptor can be closed even after original file was deleted so 
everything is save even if IOException is thrown and file is deleted before 
finally block is called.
                
> Saving IndexSummaries to disk
> -----------------------------
>
>                 Key: CASSANDRA-2392
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-2392
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Chris Goffinet
>            Assignee: Vijay
>            Priority: Minor
>             Fix For: 1.1
>
>         Attachments: 0001-re-factor-first-and-last.patch, 
> 0001-save-summaries-to-disk.patch, 0002-save-summaries-to-disk-v2.patch, 
> 0002-save-summaries-to-disk-v3.patch, 0002-save-summaries-to-disk.patch
>
>
> For nodes with millions of keys, doing rolling restarts that take over 10 
> minutes per node can be painful if you have 100 node cluster. All of our time 
> is spent on doing index summary computations on startup. It would be great if 
> we could save those to disk as well. Our indexes are quite large.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to