-
Notifications
You must be signed in to change notification settings - Fork 510
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
Stable 1.12 #5508
Stable 1.12 #5508
Conversation
This patch fixes a bug where an invalidated undo log might have not been properly cleaned up after a crash. Such a log would show that it has no entries, so seemingly nothing to cleanup. However, a single log can be composed of multiple allocations in a singly-linked list. Typically all allocations but the first one are deallocated as part of the cleanup process on recovery. And the log creation logic makes decisions based on that assumption. And that assumption was being violated in certain circumstanes where applications crashed in large transactions, after a commit but before the undo log was fully invalidated. Reported-by: @task3r
Right now pmem2_xyz_async family of functions don't use VDM_F_MEM_DURABLE vdm flags, they merely check if they are supported by the vdm. This might lead to a situation where a vdm supports durable writes, but pmem2 doesn't take advantage of that support, even when it should. This patch addresses that by using the durable flag whenever that's appropriate, and handling the flag correctly in pmem2 mover implementation.
obj: always perform cleanup of logs with leftover state
pmem2: make sure async functions use proper vdm flags
A recent tried to globally add a new flag in the makefiles, which inadvertently caused the build system to stop adding important compile flags like '-O2'...
common: fix default cflags in makefiles
pmem2: enable movdir64b compilation but disable it in the runtime
This patch fixes a bug where free prior to any allocs, combined with reservations, could have led to overlapping allocations - which ultimately was causing corrupted heap and incorrect statistics. This problem is caused by lazy heap runtime state reclamation. Heap runtime state is rebuilt lazily whenever required to serve allocation requests. Deallocations (free) simply update persistent metadata and, in case of huge allocations, inserts the freed chunk into a container of free chunks. On reclaim, all free chunks not already deallocated are inserted into a freelist. This would have been fine, but libpmemobj's allocator enables software to reserve a chunk, removing it from the heap runtime state without updating the persistent on-media layout. This means that software can deallocate a chunk, reserve that same chunk, allocate something normally - triggering heap zone reclamation, and then it can finally publish (actually persistently allocate) that reserved chunk. This can lead to the same chunk being potentially allocated twice... This patch fixes this problem by ensuring that object's zone is fully processed and reclaimed prior to deallocation. Reported-by: @jolivier23
obj: ensure zones are reclaimed prior to free (1.11)
Stable 1.12
f3b809217 Merge pull request pmem#110 from lplewa/master 11fddd440 masync: suppres unused parameters in future.h 1c6ba8d18 masync: 0.2.0-rc2 release 76ba3a10f Merge pull request pmem#108 from pbalcer/fix-use-after-free-membuf c2885af40 common: fix use-after-free in membuf in mt scenarios 1914de3de Merge pull request pmem#106 from DamianDuy/updateAsyncDoc ffedbde35 doc: update async property documentation 12e9fa42c Merge pull request pmem#105 from DamianDuy/addAsyncDoc cb1734572 doc: add async property to documentation 3b44a2cb9 Merge pull request pmem#100 from DamianDuy/addAsyncFlag 9f740e491 masync: add flag indicating that future is async b7216cab1 Merge pull request pmem#90 from DamianDuy/addFlushOp 01ef54aa4 masync: add flush operation for DML data mover c0c8f9ddf Merge pull request pmem#104 from lukaszstolarczuk/update-docs eb41ffcee doc: update code styling to properly appear in manpage e573f7e89 Merge pull request pmem#102 from lplewa/readback c98664d80 Merge pull request pmem#103 from pbalcer/atomic-load-ringbuf 2490b0cee common: use correct atomics in ringbuf 428ecdaf9 masync: add destination_readback flag to persistent memory writes f1203f533 Merge pull request pmem#71 from kswiecicki/masync-example-hashmap bc1d04ef7 masync: add hashmap example 352b4c1aa Merge pull request pmem#92 from lukaszstolarczuk/add-macro-is-init a6e312918 Merge pull request pmem#96 from kilobyte/no-arch-ifdefs 27e8795a8 masync: add macro to check if chain entry was initialized 790bb3522 Merge pull request pmem#99 from lukaszstolarczuk/fix-manpage-header af2e47738 common: replace improper 'RPMA' in man pages with 'MINIASYNC' 01e68810b masync: drop unused #ifdefs that block portability git-subtree-dir: src/deps/miniasync git-subtree-split: f3b809217fc6f4401c227e707708a803725290bc
Miniasync update
a8d51f0a4 Merge pull request pmem#111 from kswiecicki/kw-issues b1efa1c99 Merge pull request pmem#109 from KFilipek/add-casting dc88b63a0 masync: fix hashmap memory allocation 97055b841 masync: assert hashmap example copied value 701844c99 masync: fix hashmap example allocation checking 93b1698ff masync: fix hashmap example cleanup 7fa75e02f masync: avoid casting error on C++ compilation git-subtree-dir: src/deps/miniasync git-subtree-split: a8d51f0a46d8143cbbc5ac8ca272ad3ba19fed6e
masync update for stable 1.12
Codecov Report
@@ Coverage Diff @@
## master #5508 +/- ##
==========================================
- Coverage 72.12% 71.87% -0.26%
==========================================
Files 193 195 +2
Lines 30866 30727 -139
==========================================
- Hits 22263 22084 -179
- Misses 8603 8643 +40 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks 🆗 to me, but I'm wondering about the last commit: "Merge remote-tracking branch 'upstream/master' into stable-1.12". It seems like the master was merged into the stable branch... Commits range in this PR looks ok, so I'm not sure if we should worry 😉
Reviewed 9 of 93 files at r1.
Reviewable status: 9 of 93 files reviewed, all discussions resolved (waiting on @wlemkows)
I merged such PR into my master branch, and it looks acceptable, but I would still re-do this merge, as expected. Meaning, "the other way around" - you should rather merge a stable branch into master. Right now, after merging this PR you'd have an extra "connection" between master and stable, see magenta and blue lines on the image below: |
Well, this is correct. I created merge commit as i had to resolve conflict. GH cannon do FF merge, that's why after "merge pull reqest" thru GH interface you have a second merge commit. This is intended, but if you really don't want this you can "merge this PR by hand", and close this PR. Theoretically i could do the merge in other way, but this would change only parent order, which doesn't matter. |
I've done my share of manual "merges" and I agree, you should see the "merge" commit in here, but it should be the other way around: not "Merge remote-tracking branch 'upstream/master' into stable-1.12" but rather "Merge 'upstream/stable-1.12' into master". I believe, what you did, you executed:
and I believe, you should do:
😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better 😄 Thanks! 👍
Reviewed all commit messages.
Reviewable status: 9 of 93 files reviewed, all discussions resolved (waiting on @wlemkows)
Any update? @wlemkows can you please review/merge this PR? |
This change is