Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tx: fix PageCount update in allocate #65

Merged
merged 1 commit into from
Dec 7, 2017
Merged

Conversation

zrss
Copy link

@zrss zrss commented Nov 8, 2017

tx: PageCount add up the count that we try to allocate

@codecov-io
Copy link

codecov-io commented Nov 8, 2017

Codecov Report

Merging #65 into master will increase coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   85.56%   85.57%   +<.01%     
==========================================
  Files           9        9              
  Lines        1857     1858       +1     
==========================================
+ Hits         1589     1590       +1     
+ Misses        159      158       -1     
- Partials      109      110       +1
Impacted Files Coverage Δ
db.go 82.79% <100%> (-0.04%) ⬇️
tx.go 76.82% <100%> (+0.6%) ⬆️
freelist.go 81.7% <50%> (-1.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c6cbfb...22635d7. Read the comment docs.

@xiang90
Copy link
Contributor

xiang90 commented Nov 8, 2017

@zrss can you add a test for this fix? thanks.

@zrss
Copy link
Author

zrss commented Nov 9, 2017

@xiang90

i add an accessible method for tx.allocate, and do not close the db file with MustClose method to skip MustCheck

@gyuho
Copy link
Contributor

gyuho commented Nov 10, 2017

Why not creating another unit test file rather than exposing allocate method?

@zrss
Copy link
Author

zrss commented Nov 12, 2017

@gyuho

I added a new unit test file, but not sure whether the file name is appropriate. And there is a bolt_test package for test files, why not put them in the same package as bolt (such as freelist_test.go) ... just curious about that

@zrss zrss changed the title tx: fix the number of pages is not incorrectly counted tx: fix the number of pages is incorrectly counted Nov 12, 2017
@zrss zrss changed the title tx: fix the number of pages is incorrectly counted tx: fix the number of pages is not incorrectly counted Nov 12, 2017
@zrss zrss changed the title tx: fix the number of pages is not incorrectly counted tx: fix the number of pages is incorrectly counted Nov 12, 2017
@gyuho
Copy link
Contributor

gyuho commented Dec 7, 2017

Defer to @xiang90

@xiang90
Copy link
Contributor

xiang90 commented Dec 7, 2017

lgtm

@xiang90 xiang90 merged commit 48ea1b3 into etcd-io:master Dec 7, 2017
@gyuho gyuho changed the title tx: fix the number of pages is incorrectly counted tx: fix PageCount update in allocate Dec 7, 2017
@zrss zrss deleted the fix-page-cnt branch December 7, 2017 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants