Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

posix: disable space overallocation on append & fix 16k blocks tests #432

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

marcinslusarz
Copy link
Contributor

@marcinslusarz marcinslusarz commented Oct 16, 2017

This change is Reviewable

…nd the end of file

Not caught because rw test is not executed for 16k block size.
Current block sizes are 16k, 256k and 2M, so there's no point in
allocating more than 2M.
@marcinslusarz marcinslusarz changed the title posix: disavle space overallocation on append & fix 16k blocks tests posix: disable space overallocation on append & fix 16k blocks tests Oct 16, 2017
@krzycz
Copy link
Contributor

krzycz commented Oct 16, 2017

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@codecov
Copy link

codecov bot commented Oct 16, 2017

Codecov Report

Merging #432 into master will increase coverage by 0.18%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
+ Coverage   75.95%   76.13%   +0.18%     
==========================================
  Files          70       70              
  Lines        8815     8813       -2     
  Branches     1781     1782       +1     
==========================================
+ Hits         6695     6710      +15     
+ Misses       1628     1611      -17     
  Partials      492      492
Flag Coverage Δ
#ltp_tests 44.66% <0%> (-0.09%) ⬇️
#sqlite_tests 44.88% <0%> (+0.01%) ⬆️
#tests_antool 15.05% <0%> (ø) ⬆️
#tests_posix_multi_threaded 25.36% <0%> (-0.19%) ⬇️
#tests_posix_single_threaded 53.64% <33.33%> (+0.14%) ⬆️
#tests_preload 43.89% <0%> (-0.53%) ⬇️
Impacted Files Coverage Δ
src/libpmemfile-posix/data.c 87.74% <0%> (-6.38%) ⬇️
src/libpmemfile-posix/pmemfile-posix.c 68.96% <0%> (+22.29%) ⬆️
src/libpmemfile-posix/lseek.c 90.78% <100%> (+4.69%) ⬆️
src/libpmemfile-posix/utils.c 66.66% <0%> (-1.67%) ⬇️
src/libpmemfile-posix/rename.c 90.72% <0%> (-0.41%) ⬇️
src/libpmemfile-posix/dir.c 85.29% <0%> (+0.24%) ⬆️
src/libpmemfile-posix/blocks.c 82.5% <0%> (+12.5%) ⬆️
... and 1 more

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 7f181b5...f1083db. Read the comment docs.

pfp, 1, 1, 0, (env_block_size == 0x4000) ? bc_4k : bc));

if (env_block_size == 0x4000)
EXPECT_TRUE(test_pmemfile_stats_match(pfp, 1, 1, 1, bc_4k));
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, perhaps we should rename bc_4k -> bc_16k.
Not necessarily in this patch.

@@ -191,7 +191,8 @@ lseek_seek_data_or_hole(PMEMfilepool *pfp, struct pmemfile_vinode *vinode,
if (vinode_is_regular_file(vinode)) {
if (whence == PMEMFILE_SEEK_DATA) {
offset = lseek_seek_data(pfp, vinode, offset, fsize);
ASSERT(offset <= fsize);
if (offset > fsize)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this should be offset > fsize or offset >= fsize.
E.g. what should happen with an empty file, should offset == 0 be OK with SEEK_DATA?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants