Its definitely easy to migrate to 4.4 and also maintain in the future if we don't care about naming snapshots. But I think allowing the users to name a snapshot adds more value. What do you think Aaron?
Saurabh, Did you get a chance to update the patch for https://issues.apache.org/jira/browse/BLUR-208 to include merge policy changes we discussed? - Rahul On Wed, Oct 30, 2013 at 12:56 PM, saurabh gupta <[email protected]>wrote: > 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 > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > > > >
