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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
