Issue 550: add readLastAddConfirmedAndEntry in ReadHandle for long poll read

[zhaijack]

Descriptions of the changes in this PR:
1, add a class LastAddConfirmedAndEntry and metnod readLastAddConfirmedAndEntry() in ReadHandle;
2, add implementation for readLastAddConfirmedAndEntry in LedgerHandler;
3, add testcase in BookKeeperApiTest;
4, remove un-used imports, break down long lines, fix wrong comments.

---

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

• Make sure the PR title is formatted like:
  <Issue # or BOOKKEEPER-#>: Description of pull request
e.g. Issue 123: Description ...
e.g. BOOKKEEPER-1234: Description ...
• Make sure tests pass via mvn clean apache-rat:check install findbugs:check.
• Replace <Issue # or BOOKKEEPER-#> in the title with the actual Issue/JIRA number.

---

[asfgit]

FAILURE

--none--

[eolivelli]

I think it is better that this class is not an inner class but a top level class

[sijie]

move it out of ReadHandle.java, make it an interface

interface LastConfirmedAndEntry extends AutoCloseable {

     long getLastAddConfirmed();

     boolean hasEntry();

     LedgerEntry getEntry();

}

create a class LastConfirmedAndEntryImpl to implement the interface.

[zhaijack]

Thanks @eolivelli @sijie , will change it.

[eolivelli]

in case of THIS timeout the CompletableFuture will return null ? or is will be completed exceptionally ?

[sijie]

it will return null entry and latest lac on timeout.

[zhaijack]

Yes. In LedgerHandle, most of the similar situation will return null.

[eolivelli]

Can we add a line which explains? That was my real request, sorry

[eolivelli]

Can we add a line which explains? That was my real request, sorry

[zhaijack]

Thanks. sure will add it.

[asfgit]

FAILURE

--none--

[asfgit]

FAILURE

--none--

[sijie]

it is not needed.

[zhaijack]

Thanks. It is originally there, this change adjust the order of importing, will handle it.

[sijie]

move it out of ReadHandle.java, make it an interface

interface LastConfirmedAndEntry extends AutoCloseable {

     long getLastAddConfirmed();

     boolean hasEntry();

     LedgerEntry getEntry();

}

create a class LastConfirmedAndEntryImpl to implement the interface.

[sijie]

it will return null entry and latest lac on timeout.

[asfgit]

FAILURE

--none--

[sijie]

you don't this import

[zhaijack]

right. will remove it.

[sijie]

no space before '}'

[zhaijack]

Thanks, will change it.

[sijie]

no space before '}'

[zhaijack]

Thanks, will change it.

[sijie]

no space before '}'

[zhaijack]

Thanks, will change it.

[sijie]

I think you can simplify this class using lombok. you don't need to write constructor and getters.

[zhaijack]

Yes. I got your idea, but since this is an implementation for interface, explict these method seems more easy to read.

[sijie]

space before '{'

[jvrao]

LGTM

[asfgit]

FAILURE

--none--

[sijie]

LGTM.

@jiazhai can you address @eolivelli 's comment on timeoutMillis? then we can move forward.

[jiazhai]

@sijie , thanks. have added it.

[asfgit]

FAILURE

--none--

[eolivelli]

Sorry @jiazhai maybe the test could become flaky. Do you think we can enhance it

@ivankelly @sijie

[eolivelli]

We have a 999 timeout less then 1 second. This test could be flaky.
Maybe we can do a loop until the condition is verified

[jiazhai]

@eolivelli Thanks.
This is only for test the interface works, There are other tests for the functionality of asyncReadLastConfirmedAndEntry().
Here it read an old entry, which has already been there, so ideally it need no wait time.

[eolivelli]

Sorry @jiazhai maybe the test could become flaky. Do you think we can enhance it

@ivankelly @sijie

[eolivelli]

+1 looks good