Skip to content

Fix batch read data in cache does not take effect#2231

Merged
eolivelli merged 1 commit intoapache:masterfrom
liudezhi2098:issue-branch-xxx
Apr 13, 2020
Merged

Fix batch read data in cache does not take effect#2231
eolivelli merged 1 commit intoapache:masterfrom
liudezhi2098:issue-branch-xxx

Conversation

@liudezhi2098
Copy link
Copy Markdown
Contributor

@liudezhi2098 liudezhi2098 commented Dec 31, 2019

Descriptions of the changes in this PR:

Motivation

Fix batch read data in cache does not take effect

Changes

(Describe: what changes you have made)

Master Issue: #2230
Fixes: #2230

@eolivelli
Copy link
Copy Markdown
Contributor

Hi.
Can you please explain your problem and the fix?

We should also add a test case to demonstrate that the fix works and in order to prevent regressions in the future.

Thank you for contributing your fix

@liudezhi2098
Copy link
Copy Markdown
Contributor Author

liudezhi2098 commented Jan 2, 2020 via email

@liudezhi2098
Copy link
Copy Markdown
Contributor Author

retest this please

@liudezhi2098 liudezhi2098 requested a review from jiazhai January 7, 2020 07:33
@jiazhai
Copy link
Copy Markdown
Member

jiazhai commented Jan 7, 2020

rebuild java11
run integration tests

@liudezhi2098 liudezhi2098 requested a review from jiazhai January 7, 2020 11:25
@jiazhai
Copy link
Copy Markdown
Member

jiazhai commented Jan 8, 2020

@eolivelli is helping fix the CI issue.

Copy link
Copy Markdown
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I am working on CI.

I wonder if we can add some unit test, even using mockito

@sijie
Copy link
Copy Markdown
Member

sijie commented Jan 26, 2020

@eolivelli

I think the code is obvious and the code path should have been already covered by existing tests. So this change should be good to go.

Copy link
Copy Markdown
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

#shipit

@liudezhi2098
Copy link
Copy Markdown
Contributor Author

retest this please

@eolivelli
Copy link
Copy Markdown
Contributor

I am sorry but the magic comment doesn't work anymore with github actions.
I will trigger CI manually

@eolivelli
Copy link
Copy Markdown
Contributor

@liudezhi2098 can you please merge with master branch so that CI kicks in again ?
We had problems with GitHub actions, hopefully they have been solved

thanks in advance

Ghatage pushed a commit to Ghatage/bookkeeper that referenced this pull request Jun 19, 2020
Descriptions of the changes in this PR:



### Motivation

Fix batch read data in cache does not take effect

### Changes

(Describe: what changes you have made)

Master Issue: apache#2230 
Fixes: apache#2230 




Reviewers: Jia Zhai <zhaijia@apache.org>, Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <None>

This closes apache#2231 from liudezhi2098/issue-branch-xxx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bookie reads messages in batches very slowly, it takes about 300 milliseconds to read 100

4 participants