incubator-blur-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rahul challapalli <challapallira...@gmail.com>
Subject Re: Help needed for BLUR-208
Date Thu, 31 Oct 2013 16:20:53 GMT
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 <saurabh.b85@gmail.com>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 <saurabh.b85@gmail.com
> >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 <
> > challapallirahul@gmail.com> 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" <amccurry@gmail.com> 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 <
> >> > challapallirahul@gmail.com> wrote:
> >> >
> >> > > Comment/Stub out the non-compiling code and provide the patch.
> Thanks.
> >> > >
> >> > > - Rahul
> >> > > On Oct 12, 2013 1:32 PM, "saurabh gupta" <saurabh.b85@gmail.com>
> >> 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 <
> >> > > > challapallirahul@gmail.com> 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 <
> >> amccurry@gmail.com>
> >> > > > wrote:
> >> > > > >
> >> > > > > > On Tue, Oct 8, 2013 at 11:35 PM, rahul challapalli
<
> >> > > > > > challapallirahul@gmail.com> 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
<
> >> > amccurry@gmail.com>
> >> > > > > > 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 <
> >> > > > > saurabh.b85@gmail.com
> >> > > > > > > > >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 <
> >> > > > amccurry@gmail.com>
> >> > > > > > > > wrote:
> >> > > > > > > > >
> >> > > > > > > > > > On Mon, Oct 7, 2013 at 4:54 PM,
saurabh gupta <
> >> > > > > > saurabh.b85@gmail.com
> >> > > > > > > >
> >> > > > > > > > > > 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
> >> > > > > > > > > > >
> >> > > > > > > > > >
> >> > > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message