Use OptionsLogFormatter for Options::Dump #719
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This includes both the CMake and Makefile configurations, various scripts that are invoked during the build or during test runs, as well as the Java native library loader code.
This includes simple renaming from `rocksdb` to `speedb` as well as replacing URLs that are pointing to the RocksDB repository with the URL of the Speedb repository.
…peedb-io#64) This is done across all tools except trace_replay, which specifically looks for the RocksDB version for database compatibility checks.
The release pipeline is currently editing the RocksDB version and in a broken manner. Fix it and use a proper trigger for the artefacts release.
…edb-io#202) In the case where we create the test path, we should clean up after the run finishes as done in the Makefile, due to garbage that is left behind by tests and accumulates. While at it, randomise test scheduling in order to increase the likelihood of surfacing latent bugs that are hidden by the execution order.
The build tag can be set during the build (using either the Makefile or the CMake). If it's not provided, and we're not in a release build, it will be calculated using the state of the git tree since the last release tag (for example, for this PR the build tag will be calculated as `(main+17)-(156-build-add-a-build-tag-into-the-version-for-non-release-builds+1)`. If the git tree state can not be determined, a question mark will be used instead.
Add support for detecting and using ccache/sccache to the Makefile build in order to speed up build times as it's done in the CMake configuration.
…o#144) This is a continuation of speedb-io#126 with changes that were missed there in case a CF drop was encountered after the flush was completed but before or during manifest write. Rolling back on CF drop is meaningless because a flush is not possible in that state, and the only path forwards is for the memtables to be freed shortly afterwards, so making them available for flush again is useless at best. Additionally, this might be needed for the WriteBufferManager changes in in the future for triggering flushes based on immutable memory as well, and rolling back flushes causes the memtable memory to become ready for flush again for a brief period of time until it is dropped, which might wrongly affect the WBM decisions. Finally, the code installs a new version even if no changes are made. This is unnecessary and as such that part is moved into the version-mutating code path only. This PR also adds two regression tests, so we could rely on rollback not being performed on CF drop.
…speedb-io#217) In speedb-io#194 the default value for background compaction threads was changed to 8 (from 1). This caused some tests to fail and fixes were implemented as part of speedb-io#197. Alas, it seems that the fixes weren't complete, as under stress a test still fail. Make it so the test is truly fixed now. This can be verified by running them in a loop with the machine overloaded: ``` $ while ./deletefile_test --gtest_filter=DeleteFileTest.BackgroundPurgeCFDropTest; do sleep 1; done ```
Both were incorrectly referencing the old artefact names before the changes in speedb-io#66.
Specify `SPDB_RELEASE_BUILD` to both CMake and the Makefile builds in order to get the release artefacts to build without a build tag of `?`.
…peedb-io#225) Merge commits were incorrectly handled due to the named use of `git rev-list`, which does not give an ancestry path as I expected, but simply a list of unique commits between the two refs. Additionally, the check for the base tag was wrong in the face of merge commits, since a tag could be an ancestor through a merge commit, but not be the actual base of the branch. Both of these issues resulted in an incorrect tag being calculated, as well as a build slowdown due to the iteration over all of the commits from the other parent of the merge commit, which is unneeded. While at it, add logic for caching the build tag in order to speed up builds when the checked-out git tree hasn't changed.
fix by adding WaitUntilSleeping before deleting the iterator
Import shorten memtable switch latency (speedb-io#14)
…edb-io#748) * added test that supports commits * renamed the workflow
… an existing RocksDB file (speedb-io#744)
* Expose Options::periodic_compaction_seconds via C API * update HISTORY --------- Co-authored-by: zaidoon <[email protected]>
…y that may cause the method to return an incorrect answer (speedb-io#759)
speedb-io#651) This PR adds an OptionsFormatter to the ConfigOptions. This class allows the options to be written and read in different formats. Three formats initially supported for write are the existing Options-file properties format, the ";"-separated list of items, and writing the options to the log file. By adding a LogOptionsFormatter, the system will be able to automatically log options to the file as they are added or removed. Currently, this involves a secondary step of updating the Dump method which is sometimes missed. Furthermore, some sub-classes may be skipped or not have their options printed at all.
|
@mrambacher can you please solve conflicts so I can review it? |
* renamed ci to build workflows * ci_pipeline - added new jobs with reusable workflows with build tests on windows, macos and ubuntu arm * ci_pipeline - added new jobs with reusable workflows with build tests on windows, macos and ubuntu arm * switching from reusable workflows to local, added build and tests for Windows, MacOS and Ubuntu on Arm to the ci_workflow * commented out arena_test in the windows workflow * Renamed some jobs * Renamed some jobs * commented out the test for Windows, leaving build only * re-arranges the ci workflow and added a summary job which will be made as required check * updated condition of build and other jobs that come after check license * updated condition of build and other jobs that come after check license * updated condition of build and other jobs that come after check license * fix instructions for macos_Arm * upgraded gh cache action from v2 to v3.3.2 * merged the updated check license and history workflow and disabled condition for it execution from ci_pipeline that made sure it is running only on PR review * switched github.ref_name to github.ref for Linux on Arm build * added a test that checks the workflow trigger and sets up the correct branch for building on Linux Arm, this test is needed for docker container that we use for the build * added Dump GitHub context step to Linux-Arm-build * added case pull_request_review for build on Linux Arm * added case pull_request_review for build on Linux Arm * added case pull_request_review for build on Linux Arm --------- Co-authored-by: speedbadmin <[email protected]>
|
This PR fixes #692 |
And make GetOptionString also call SerializePrintableOptions.
|
lgtm |
ofriedma
previously approved these changes
Nov 26, 2023
|
I manually tested the ARM compile, it worked fine |
mrambacher
dismissed
ofriedma’s stale review
The merge-base changed after approval.
|
GitHub Runner Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces the code in the Options::Dump method with code using the Options infrastructure. A new "SerializePrintableOptions" method was added to the Configurable class which allows the values not serializable by the options to be included in the output. A ConfigOptions::Depth::KPrintable was added to tell the classes to invoked this method as part of the serialization process.
The Options::Dump methods now use the this infrastructure to generate their output. The code in the GetPrintableOptions methods of various was either deleted (because the values were serialized as Options) or moved into the SerializePrintableOptions method.