build: upgrade abseil and fix serde breakage #15249
Merged
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
|
did you mean to have 1e61d66 in there? |
dotnwat
force-pushed
the
upgrade-abseil
branch
from
c66f615
to
203f58c
Compare
no i just dropped it not sure how that snuck in there |
dotnwat
commented
Nov 30, 2023
| @@ -366,7 +367,7 @@ cluster_recovery_backend::find_controller_snapshot_in_bucket( | |||
| const size_t snap_size = co_await reader.get_snapshot_size(); | |||
| auto snap_buf_parser = iobuf_parser{ | |||
| co_await read_iobuf_exactly(reader.input(), snap_size)}; | |||
| snapshot = serde::read<cluster::controller_snapshot>( | |||
| snapshot = co_await serde::read_async<cluster::controller_snapshot>( | |||
There was a problem hiding this comment.
this is necessary because the synchronous versions that had been using reflection no longer function.
There was a problem hiding this comment.
Side note, but it would be great to somehow explicitly mark sync serde forbidden for the snapshot struct and not rely on is_standard_layout implicitly being false.
rockwotj
previously approved these changes
Nov 30, 2023
ztlpn
reviewed
Nov 30, 2023
| template<typename T> | ||
| requires std::is_standard_layout_v<T> |
There was a problem hiding this comment.
can't we just do something like requires reflection::to_tuple(T{})? That would be easier to understand IMO.
dotnwat
force-pushed
the
upgrade-abseil
branch
from
203f58c
to
32a6e67
Compare
Upgraded abseil containers cause containing types to no longer have standard layout, so reflection doesn't work. Signed-off-by: Noah Watkins <[email protected]>
Signed-off-by: Noah Watkins <[email protected]>
This test depended on types being both async and sync serde enabled. However, this is no longer the case with the changes from abseil and its affect on standard layout of types. Signed-off-by: Noah Watkins <[email protected]>
Also moves the dependency to be earlier in the deps list since it is so foundational. Signed-off-by: Noah Watkins <[email protected]>
rockwotj
previously approved these changes
Dec 1, 2023
ztlpn
previously approved these changes
Dec 1, 2023
|
Unit test failure was #13275 |
New checks in abseil revealed: 13: [/home/nwatkins/src/redpanda/vbuild/debug/clang/rp_deps_install/include/absl/container/internal/raw_hash_set.h : 1259] RAW: Invalid iterator comparison. Comparing iterator with an end() iterator from a different hashtable. 13: ==1795828==WARNING: ASan is ignoring requested __asan_handle_no_return: stack type: default top: 0x7fa25cf16000; bottom 0x7ffd07b05000; size: 0xffffffa555411000 (-389411696640) 13: False positive error reports may follow 13: For details see google/sanitizers#189 13: unknown location(0): fatal error: in "complex_uuid_types_test": signal: SIGABRT (application abort requested) Signed-off-by: Noah Watkins <[email protected]>
|
Fixed a bug in the uuid unit test that the abseil upgrade revealed. |
rockwotj
approved these changes
Dec 1, 2023
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.
Upgrade abseil and fix the resulting serde breakage.
Corresponding vtools PR to see the build result https://github.com/redpanda-data/vtools/pull/2372.
Backports Required
Release Notes