Hi Rahul/Aaron, I was busy in some other work so I did not get time to look on this story. But yesterday I think about the approach that we should take for SnapshotDeletionPolicy. If we want to keep the name of snapshot as user might need to give the name to them then we have to persist the mapping of name of snapshot and corresponding generation somewhere in our system. My opinion is we should go with generation rather keeping name of snapshot.
Please provide your feedback. Thanks, Saurabh Gupta On Sat, Oct 12, 2013 at 11:22 PM, saurabh gupta <[email protected]>wrote: > Hi Rahul, > > I have created sub tasks which are relevant to the story. Add more sub > tasks if you think the story should have. I have attached the patch within > the story. Review the changes and let me know if I can commit the changes > into branch or not. > > Please provide the link for branch as well. > > Thanks, > Saurabh > > > On Sat, Oct 12, 2013 at 8:51 PM, rahul challapalli < > [email protected]> wrote: > >> Makes total sense Aaron. I was not sure whether to commit broken code into >> a repo. Thanks for your response. >> >> - Rahul >> On Oct 12, 2013 2:35 PM, "Aaron McCurry" <[email protected]> wrote: >> >> > For what it's worth, I would prefer that things that are not compiling >> to >> > be left in an error state while we are working on a patch. Especially >> in >> > branches like this where we are working together to resolve issues. The >> > biggest reason is because they can be easily identified and worked on, >> if >> > they are commented out we would have to rely on @TODO or something else >> to >> > find the issues. >> > >> > So for the record, I'm perfectly ok with committing broken code into >> > feature branches. Just make a note in the commit that it's broken. >> > >> > Thanks! >> > >> > Aaron >> > >> > >> > On Sat, Oct 12, 2013 at 1:38 PM, rahul challapalli < >> > [email protected]> wrote: >> > >> > > Comment/Stub out the non-compiling code and provide the patch. Thanks. >> > > >> > > - Rahul >> > > On Oct 12, 2013 1:32 PM, "saurabh gupta" <[email protected]> >> wrote: >> > > >> > > > Hi Rahul, >> > > > >> > > > This is a good idea that you will create a branch. I will create sub >> > task >> > > > in story. Yes I have changed code. But BlurNRT has compilation error >> > due >> > > to >> > > > snapshotdeletionpolicy so suggest me whether I should create a >> patch or >> > > > not. >> > > > >> > > > Thanks, >> > > > Saurabh >> > > > >> > > > >> > > > On Fri, Oct 11, 2013 at 10:07 PM, rahul challapalli < >> > > > [email protected]> wrote: >> > > > >> > > > > Hi Sourabh, >> > > > > >> > > > > I will create a branch for this over the weekend. Can you kindly >> > > create a >> > > > > sub-task to make snapshots work with lucene-4.4? >> > > > > We can work on this in small steps. If you have some work done on >> > this, >> > > > > provide a patch and someone will be able to review and apply it. >> Let >> > me >> > > > > know what you think. >> > > > > >> > > > > You can use this link to create patches >> > > > > >> > > >> http://ariejan.net/2009/10/26/how-to-create-and-apply-a-patch-with-git/ >> > > > > >> > > > > - Rahul >> > > > > >> > > > > >> > > > > On Thu, Oct 10, 2013 at 5:44 PM, Aaron McCurry < >> [email protected]> >> > > > wrote: >> > > > > >> > > > > > On Tue, Oct 8, 2013 at 11:35 PM, rahul challapalli < >> > > > > > [email protected]> wrote: >> > > > > > >> > > > > > > Thanks Saurabh for clarifying. >> > > > > > > >> > > > > > > Looks like we have to modify the existing code to make it work >> > with >> > > > > > Lucene >> > > > > > > 4.4 >> > > > > > > >> > > > > > > I see 2 approaches that we can take here : >> > > > > > > >> > > > > > > 1. We can write our own SanpshotDeletionPolicy if we want >> users >> > > to >> > > > be >> > > > > > > able to give names to snapshots (I can imagine people using >> date >> > as >> > > > > part >> > > > > > of >> > > > > > > the name) >> > > > > > > >> > > > > > >> > > > > > I would prefer this option, the writer is very heavy weight as >> > Rahul >> > > > has >> > > > > > stated. >> > > > > > >> > > > > > >> > > > > > > 2. We can use PersistentSanpshotDeletionPolicy. The only >> reason >> > > we >> > > > > did >> > > > > > > not use it was because it uses IndexWriter for persisting >> which >> > is >> > > a >> > > > > very >> > > > > > > heavy object. >> > > > > > > >> > > > > > > >> > > > > > > I guess we should create a separate branch for this and also >> add >> > a >> > > > > > subtask >> > > > > > > for making snapshots work. Would like to hear some thoughts >> from >> > > > others >> > > > > > as >> > > > > > > well. Thanks >> > > > > > > >> > > > > > >> > > > > > Sounds good. >> > > > > > >> > > > > > >> > > > > > > >> > > > > > > - Rahul >> > > > > > > >> > > > > > > >> > > > > > > On Tue, Oct 8, 2013 at 7:23 PM, Aaron McCurry < >> > [email protected]> >> > > > > > wrote: >> > > > > > > >> > > > > > > > Hmm, I see what you saying let me take a closer look at it >> and >> > > > report >> > > > > > > back. >> > > > > > > > >> > > > > > > > Aaron >> > > > > > > > >> > > > > > > > >> > > > > > > > On Tue, Oct 8, 2013 at 12:48 PM, saurabh gupta < >> > > > > [email protected] >> > > > > > > > >wrote: >> > > > > > > > >> > > > > > > > > Hi, >> > > > > > > > > >> > > > > > > > > In BlurNRT class there is a code which loads the existing >> > > > > snapshots: >> > > > > > > > > >> > > > > > > > > if (snapshotsDirectoryExists()) { >> > > > > > > > > // load existing snapshots >> > > > > > > > > sdp = new >> > > > > > > > > >> > SnapshotDeletionPolicy(_tableContext.getIndexDeletionPolicy(), >> > > > > > > > > loadExistingSnapshots()); >> > > > > > > > > } else { >> > > > > > > > > sdp = new >> > > > > > > > > >> > SnapshotDeletionPolicy(_tableContext.getIndexDeletionPolicy()); >> > > > > > > > > } >> > > > > > > > > >> > > > > > > > > But now in 4.4 version there is no constructor with second >> > > > > argument. >> > > > > > > They >> > > > > > > > > changed it corresponding to >> > > > > > > > > LUCENE-4973< >> http://issues.apache.org/jira/browse/LUCENE-4973 >> > > >> > > > > > > > > . >> > > > > > > > > >> > > > > > > > > Also to open a old snapshot the >> > > > > > > > > >> > > > > > > > > IndexCommit snapshot = snapshotter.getSnapshot(name); >> > > > > > > > > >> > > > > > > > > changed to >> > > > > > > > > >> > > > > > > > > IndexCommit snapshot = snapshotter.getIndexCommit(long >> gen); >> > > > > > > > > >> > > > > > > > > which takes generation >> > > > > > > > > >> > > > > > > > > I am not getting how to get the generation. >> > > > > > > > > >> > > > > > > > > I hope you understand what I am trying to say. >> > > > > > > > > >> > > > > > > > > Thanks, >> > > > > > > > > Saurabh Gupta >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > On Tue, Oct 8, 2013 at 2:54 AM, Aaron McCurry < >> > > > [email protected]> >> > > > > > > > wrote: >> > > > > > > > > >> > > > > > > > > > On Mon, Oct 7, 2013 at 4:54 PM, saurabh gupta < >> > > > > > [email protected] >> > > > > > > > >> > > > > > > > > > wrote: >> > > > > > > > > > >> > > > > > > > > > > Hi >> > > > > > > > > > > >> > > > > > > > > > > I am looking an improvement BLUR-208. I am stuck at >> one >> > > place >> > > > > in >> > > > > > > > > BlurNRT >> > > > > > > > > > > class where we are loading previous snapshots and set >> in >> > > > > > > > > > > SnapshotDeletionPolicy. But now there is no way to >> load >> > > > > previous >> > > > > > > > > > snapshots. >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > We haven't built a way to load in previous snapshots. >> We >> > > have >> > > > > > > planned >> > > > > > > > on >> > > > > > > > > > doing so but have not actually implemented it yet. We >> left >> > > the >> > > > > > > > snapshots >> > > > > > > > > > incomplete because at the time we were trying to write >> an >> > > > > > InputFormat >> > > > > > > > for >> > > > > > > > > > Hadoop. >> > > > > > > > > > >> > > > > > > > > > It shouldn't be too hard to do if the table is offline. >> > > Online >> > > > > > moves >> > > > > > > > to >> > > > > > > > > > previous snapshots will be tricky. >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > Also SnapshotDeletionPolicy is returning list of >> > > IndexCommit >> > > > > and >> > > > > > > > > > otherwise >> > > > > > > > > > > we have to use generation to get the specific >> > IndexCommit. >> > > > Now >> > > > > I >> > > > > > > dont >> > > > > > > > > > know >> > > > > > > > > > > how to get the generation. >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > Are you asking about how it works in Lucene or in Blur? >> In >> > > > Blur >> > > > > we >> > > > > > > > > create >> > > > > > > > > > a snapshot label to manage the snapshots. >> > > > > > > > > > >> > > > > > > > > > The Lucene basic code needed to open an old snapshot >> would >> > be >> > > > > > > something >> > > > > > > > > > like: >> > > > > > > > > > >> > > > > > > > > > IndexCommit snapshot = >> snapshotter.getSnapshot(name); >> > > > > > > > > > >> > > > > > > > > > DirectoryReader.open(snapshot); >> > > > > > > > > > If you could let us know what / how you would expect >> Blur >> > to >> > > > > behave >> > > > > > > > when >> > > > > > > > > > loading an old snapshot that would be great. Real world >> > use >> > > > > cases >> > > > > > > are >> > > > > > > > > the >> > > > > > > > > > best to work toward. >> > > > > > > > > > >> > > > > > > > > > Thanks! >> > > > > > > > > > >> > > > > > > > > > Aaron >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > > Can anyone help me? >> > > > > > > > > > > >> > > > > > > > > > > Thanks >> > > > > > > > > > > Saurabh >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> > >
