mmapstorage: store the OPMPHM #3136
Conversation
|
Build failed because of docker: I don't know if this is a known problem but seems unrelated to the PR. |
|
Thank you for creating this PR! Good to see activity from you 💖 |
|
@markus2330 I ran into #2130 once again. Since the addition of OPMPHM to mmap, I'd need the The implementations are "entangled" either way, since mmapstorage needs knowledge of any structures it stores. The alternative to exporting the symbols is to copypaste the What approach to you like more: exporting vs copypasta? |
|
Now the jenkins builds have finished successfully, but the status is not reported to github. 🤷♂ |
That happened in one of my PRs (a long time ago) too. I just restarted the build when it happened. For obvious reasons – Jenkins build fails all the time and takes forever – this is not an ideal solution. Anyway, I would consider this PR “ready to merge”. |
|
In some cases it is unfortunately necessary to open a new PR. Jenkins build are now hopefully faster as we now got one more server. |
|
jenkins build libelektra please |
3 similar comments
|
jenkins build libelektra please |
|
jenkins build libelektra please |
|
jenkins build libelektra please |
|
@markus2330 this should be done, but the tests are constantly failing because zeromq or gopts tests are timing out. |
|
The zeromq tests can be disabled (as nobody is working on this). Can you please report the gopts issue, maybe @kodebach finds a solution. |
|
jenkins build libelektra please |
|
Please document in doc/todo/TESTING which tests were disabled (ideally also add commit SHA-1 how to revert the disabling of tests). |
|
Seems like |
…re, add OPMPHM format flag, add superficial test for OPMPHM storage
…r magic data and increase MMAP_MINSIZE
…mprove documentation
…from non-OPMPHM build
|
@markus2330 done. |
| @@ -13,6 +13,7 @@ | |||
| #include <kdblogger.h> | |||
| #include <kdbmacros.h> | |||
| #include <kdbopmphm.h> | |||
| #include <kdbprivate.h> | |||
There was a problem hiding this comment.
Please add the reason of this include as comment next to it.
| @@ -10,6 +10,7 @@ | |||
| #include <kdbhelper.h> | |||
| #include <kdblogger.h> | |||
| #include <kdbopmphmpredictor.h> | |||
| #include <kdbprivate.h> | |||
There was a problem hiding this comment.
Please add the reason of this include as comment next to it.
| @@ -99,7 +124,12 @@ typedef struct _mmapFooter MmapFooter; | |||
| /** | |||
| * Mmap information header | |||
| * | |||
| * shall contain only fixed-width types | |||
| * Changes to this struct can/will break compatibility with existing files. | |||
| * If a breaking changed needs to be introduced, change `ELEKTRA_MAGIC_MMAP_NUMBER` | |||
There was a problem hiding this comment.
Please give details what a breaking change is:
- reordering of struct?
- adding to struct?
- ...
|
Thank you, great work! I am looking forward to see the benchmarks! |
Store the OPMPHM and generally improve mmapstorage (some critical fixes and sanity checks).
Disable
testmod_zeromqsend.Increase Jenkins CI timeout #3157.
Basics
These points need to be fulfilled for every PR:
(added as entry in
doc/news/_preparation_next_release.mdwhichcontains
_(my name)_)Please always add something to the release notes.
(first line should have
module: short statementsyntax)close #X, are in the commit messages.If you have any troubles fulfilling these criteria, please write
about the trouble as comment in the PR. We will help you.
But we cannot accept PRs that do not fulfill the basics.
Checklist
Check relevant points but please do not remove entries.
For docu fixes, spell checking, and similar none of these points below
need to be checked.
(not in the PR description)
Review
Reviewers will usually check the following:
Labels
If you are already Elektra developer:
say that everything is ready to be merged.