2017-07-24 22:54 GMT+02:00 Sijie Guo <guosi...@gmail.com>:

> On Mon, Jul 24, 2017 at 1:29 PM, Enrico Olivelli <eolive...@gmail.com>
> wrote:
>
> > Il lun 24 lug 2017, 19:39 Sijie Guo <guosi...@gmail.com> ha scritto:
> >
> > > On Mon, Jul 24, 2017 at 5:22 AM, Enrico Olivelli <eolive...@gmail.com>
> > > wrote:
> > >
> > > > Hi,
> > > > we are going to release 4.5 soon, in this release we changed
> > > > EnsemblePlacementPolicy interface.
> > > >
> > > > It you are using a "custom" EnsemblePlacementPolicy in 4.4 and you
> just
> > > > switch the bookeeeper-server JAR on the classpath your application
> > won't
> > > > run.
> > > >
> > > > I already got that problem and I needed to implement some "support
> 4.5"
> > > > option in my projects which essentially tells "do not use a custom
> > > policy",
> > > > this is very bad because there is no way to apply the custom rules
> > > required
> > > > by the project.
> > > >
> > >
> > > Do you mean the new methods introduced in placement policy? Or methods
> > that
> > > whose signatures are changed?
> > >
> >
> > The new signatures
> >
> > >
> > > I believe the new methods 'reorderSequence' is disabled by default
> unless
> > > you enable it explicitly.
> > >
> > > For methods that whose signatures were changed, we can add the old
> > methods
> > > back and create overrides to keep the binary backward compatible.
> > >
> >
> > I don't know if this cam work. I have already tried. I will double check.
> > Anyway it will be a bit messy
> >
> > >
> > > However I am not sure if this works because the placement policy is
> > > instantiated and loaded by reflection. Typically when you upgrade a
> > > version, you have to compile it first, no?
> > >
> >
> > I have several libraries which use bk and they are built on 4.4 and they
> > are working together in the same classpath.
> > For instance now I am going to change one of them in order to leverage a
> > new 4.5  feature, like readUnconfirmedEntries just as an example, so I
> need
> > to switch to 4.5 on the classpath but the other projects won't run.
> >
>
> Are you talking about mixing 4.4 and 4.5 together?
>

Yes, as for most widely used libraries (like ZooKeeper) it is common to put
a "new version" or the library on a application compiled for a previous
version.
I see that sometimes it is not possible, and for this reason we use 'major
versions' vs 'minor versions'.
When possible I think it is best not to introduce such breaking changes.





>
>
> > For the server side this is not a problem but on the client it is a real
> > production problem.
> >
>
> Can you clarify more specifically on this? Not very sure I understand your
> problem and what are you going to achieve.
>
>
Usually you have full control on bookies, but it is not always possible to
have full control of applications assembler from different components.
Example:
- I have Lib 1 v1 which is compiled with BK 4.4
- I have Lib 2 v1 which is compiled with BK 4.4
- I have App which depends on Lib1 + Lib2
- Now Lib 2 v2 need BK 4.5 for a fresh new feature
- New version of the App v2 will depend on Lib 1 v1 + Lib 2 v2 and will
need to include BK 4.5, but this won't work
Assemblers for App v2 do not have control on Lib1 and Lib2

For instance I got into this trouble while integrating my new project which
uses readUnconfirmedEntries inside an App which is compiled for BK 4.4 and
uses a custom placement policy.

So, now that we are breaking things I would like to break them in a way
that in the future will be simpler to handle

for instance:

public interface EnsemblePlacementPolicy {


    public EnsemblePlacementPolicy initialize(ClientEnvironment env);

    public void uninitalize();

    public Set<BookieSocketAddress>
onClusterChanged(Set<BookieSocketAddress> writableBookies,

Set<BookieSocketAddress> readOnlyBookies);

    public List<BookieSocketAddress> newEnsemble(LedgerSpecs ledgerSpecs,
Set<BookieSocketAddress> excludeBookies) throws BKNotEnoughBookiesException;


    public BookieSocketAddress replaceBookie(LedgerSpecs ledgerSpecs,
Collection<BookieSocketAddress> currentEnsemble,
        BookieSocketAddress bookieToReplace, Set<BookieSocketAddress>
excludeBookies) throws BKNotEnoughBookiesException;

    public List<Integer> reorderReadSequence(List<BookieSocketAddress>
ensemble,
                                             List<Integer> writeSet,
Map<BookieSocketAddress, Long> bookieFailureHistory);


    public List<Integer> reorderReadLACSequence(List<BookieSocketAddress>
ensemble,
                                                List<Integer> writeSet,
Map<BookieSocketAddress, Long> bookieFailureHistory);

    default public void updateBookieInfo(Map<BookieSocketAddress,
BookieInfo> bookieInfoMap) {
    }
}


where ClientEnvironment and LedgerSpecs contain all of the data which need
to be passed to the policy.
We can do even more better and group the other parameters

we can evaluate to switch to an abstract class, or provide an official
"Adapter", but DefaultEnsemblePlacementPolicy is already a good "base"



>
>
> > I am ok with changing the API, I was the first to ask to change it in 4.5
> > in order to access custom metadata, but when we do such things I would
> like
> > to design a future proof API.
> >
> > I know it needs some time to design and I really to not want to block 4.5
> > release.
> > I will send a BP and maybe we can discuss on a concrete proposal.
> >
>
> Can we first make things clear here before any proposals?
>

yep, sure.
I meant that writing all in a Wiki page makes it simpler for reviews and
comments, in Kafka now they works this way: create KIP -> discussion ->
adjustments -> vote
this is another problem, to be discussed in another email thraed.

>
>
> > I think this change will break DL too.
> >
>
> When we upgraded the BK in DL, we will make some code changes to make sure
> binary compatible.
>


I have referred to DL because it seems to me that it is explicit in DL
documentation that you can provide your own PlacementPolicy, so users which
implemented their own policy in DL will need to recompile.
In this specific case it it not really a problem because DL did not work
with BK 4.4 at all, but it was just an example of a third party library
which depends on BK


>
>
> > Does this sound good to you?
> >
> > Enrico
> >
> >
> > >
> > > >
> > > > As we are going to break the API now, would it be good to create a
> more
> > > > future-proof API ?
> > >
> > > We can just post-pone the problem to 4.6
> > > >
> > > > Thoughts ?
> > > >
> > > > Enrico
> > > >
> > >
> > --
> >
> >
> > -- Enrico Olivelli
> >
>

Reply via email to