Skip to content
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.

Sign up for GitHub

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

Jump to bottom

net: implement load_balancing_algorithm::fixed #12

Closed
wants to merge 1 commit into from
Closed

net: implement load_balancing_algorithm::fixed #12

wants to merge 1 commit into from

Conversation

cyx1231st
Copy link
Member

Add a new load_balancing_algorithm::fixed to force server_socket to accept connected_socket on a fixed CPU.

@cyx1231st cyx1231st mentioned this pull request Jan 14, 2020
Merged
3 tasks
rzarzynski
rzarzynski reviewed Jan 14, 2020
View reviewed changes
Copy link

@rzarzynski rzarzynski left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I would advice to split the PR into two commits:

  • the clean-up,
  • introduction of load_balancing_algorithm::fixed

and send the former one without waiting on the latter.

src/net/posix-stack.cc
auto cth = _lba == server_socket::load_balancing_algorithm::connection_distribution ?
_conntrack.get_handle() : _conntrack.get_handle(ntoh(sa.as_posix_sockaddr_in().sin_port) % smp::count);
auto cth = [this, &sa] {
switch(_lba) {
Copy link

Choose a reason for hiding this comment

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

👍 for improving the code readability.

Copy link

Choose a reason for hiding this comment

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

i'd suggest add a space after switch.

@cyx1231st
net: posix: Introduce load_balancing_algorithm::fixed
f4c62e5 
Add a new policy load_balancing_algorithm::fixed to force server_socket to
accept connected_socket on a fixed shard.

Signed-off-by: Yingxin Cheng <[email protected]>
@cyx1231st cyx1231st force-pushed the posix-loadbalance-fixed branch from 86d514f to f4c62e5 Compare January 22, 2020 08:17
@cyx1231st
Copy link
Member Author

Updated related documentation. I didn't split the commit because I think it's acceptable to use conditional ternary operator when there are only two cases, except that the line is too long and could be wrapped.

@tchaikov
Copy link

closing, as it's been upstreamed.

@tchaikov tchaikov closed this Jan 23, 2020
tchaikov pushed a commit that referenced this pull request May 13, 2021
@bhalevy @avikivity
test: closeable: allocate variables accessed by continuations using d…
4d69526 
…o_with

Fixes failures in debug mode:
```
$ build/debug/tests/unit/closeable_test -l all -t deferred_close_test
WARNING: debug mode. Not for benchmarking or production
random-seed=3064133628
Running 1 test case...
Entering test module "../../tests/unit/closeable_test.cc"
../../tests/unit/closeable_test.cc(0): Entering test case "deferred_close_test"
../../src/testing/seastar_test.cc(43): info: check true has passed
==9449==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
terminate called after throwing an instance of 'seastar::broken_promise'
  what():  broken promise
==9449==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fbf1f49f000; bottom 0x7fbf40971000; size: 0xffffffffdeb2e000 (-558702592)
False positive error reports may follow
For details see google/sanitizers#189
=================================================================
==9449==AddressSanitizer CHECK failed: ../../../../libsanitizer/asan/asan_thread.cpp:356 "((ptr[0] == kCurrentStackFrameMagic)) != (0)" (0x0, 0x0)
    #0 0x7fbf45f39d0b  (/lib64/libasan.so.6+0xb3d0b)
    #1 0x7fbf45f57d4e  (/lib64/libasan.so.6+0xd1d4e)
    #2 0x7fbf45f3e724  (/lib64/libasan.so.6+0xb8724)
    #3 0x7fbf45eb3e5b  (/lib64/libasan.so.6+0x2de5b)
    #4 0x7fbf45eb51e8  (/lib64/libasan.so.6+0x2f1e8)
    #5 0x7fbf45eb7694  (/lib64/libasan.so.6+0x31694)
    #6 0x7fbf45f39398  (/lib64/libasan.so.6+0xb3398)
    #7 0x7fbf45f3a00b in __asan_report_load8 (/lib64/libasan.so.6+0xb400b)
    #8 0xfe6d52 in bool __gnu_cxx::operator!=<dl_phdr_info*, std::vector<dl_phdr_info, std::allocator<dl_phdr_info> > >(__gnu_cxx::__normal_iterator<dl_phdr_info*, std::vector<dl_phdr_info, std::allocator<dl_phdr_info> > > const&, __gnu_cxx::__normal_iterator<dl_phdr_info*, std::vector<dl_phdr_info, std::allocator<dl_phdr_info> > > const&) /usr/include/c++/10/bits/stl_iterator.h:1116
    #9 0xfe615c in dl_iterate_phdr ../../src/core/exception_hacks.cc:121
    #10 0x7fbf44bd1810 in _Unwind_Find_FDE (/lib64/libgcc_s.so.1+0x13810)
    #11 0x7fbf44bcd897  (/lib64/libgcc_s.so.1+0xf897)
    #12 0x7fbf44bcea5f  (/lib64/libgcc_s.so.1+0x10a5f)
    #13 0x7fbf44bcefd8 in _Unwind_RaiseException (/lib64/libgcc_s.so.1+0x10fd8)
    #14 0xfe6281 in _Unwind_RaiseException ../../src/core/exception_hacks.cc:148
    scylladb#15 0x7fbf457364bb in __cxa_throw (/lib64/libstdc++.so.6+0xaa4bb)
    scylladb#16 0x7fbf45e10a21  (/lib64/libboost_unit_test_framework.so.1.73.0+0x1aa21)
    scylladb#17 0x7fbf45e20fe0 in boost::execution_monitor::execute(boost::function<int ()> const&) (/lib64/libboost_unit_test_framework.so.1.73.0+0x2afe0)
    scylladb#18 0x7fbf45e21094 in boost::execution_monitor::vexecute(boost::function<void ()> const&) (/lib64/libboost_unit_test_framework.so.1.73.0+0x2b094)
    scylladb#19 0x7fbf45e43921 in boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) (/lib64/libboost_unit_test_framework.so.1.73.0+0x4d921)
    scylladb#20 0x7fbf45e5eae1  (/lib64/libboost_unit_test_framework.so.1.73.0+0x68ae1)
    scylladb#21 0x7fbf45e5ed31  (/lib64/libboost_unit_test_framework.so.1.73.0+0x68d31)
    scylladb#22 0x7fbf45e2e547 in boost::unit_test::framework::run(unsigned long, bool) (/lib64/libboost_unit_test_framework.so.1.73.0+0x38547)
    scylladb#23 0x7fbf45e43618 in boost::unit_test::unit_test_main(bool (*)(), int, char**) (/lib64/libboost_unit_test_framework.so.1.73.0+0x4d618)
    scylladb#24 0x44798d in seastar::testing::entry_point(int, char**) ../../src/testing/entry_point.cc:77
    scylladb#25 0x4134b5 in main ../../include/seastar/testing/seastar_test.hh:65
    scylladb#26 0x7fbf44a1b1e1 in __libc_start_main (/lib64/libc.so.6+0x281e1)
    scylladb#27 0x4133dd in _start (/home/bhalevy/dev/seastar/build/debug/tests/unit/closeable_test+0x4133dd)
```

Signed-off-by: Benny Halevy <[email protected]>
Message-Id: <[email protected]>
Matan-B pushed a commit that referenced this pull request Jul 7, 2024
@tchaikov @xemul
apps/httpd: free allocated http_server_control
2b47c97 
in main(), we creates an instance of `http_server_control` using
new, but we never destroy it. this is identified by ASan

```
==2190125==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x55e21cf487bd in operator new(unsigned long) /home/kefu/dev/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:86:3
    #1 0x55e21cf6cf31 in main::$_0::operator()() const::'lambda'()::operator()() const /home/kefu/dev/seastar/apps/httpd/main.cc:121:27
    #2 0x55e21cf6b4cc in int std::__invoke_impl<int, main::$_0::operator()() const::'lambda'()>(std::__invoke_other, main::$_0::operator()() const::'lambda'()&&) /usr/lib/gcc/x86_64-redhat-linux/14/../../../../incl
ude/c++/14/bits/invoke.h:61:14
    #3 0x55e21cf6b46c in std::__invoke_result<main::$_0::operator()() const::'lambda'()>::type std::__invoke<main::$_0::operator()() const::'lambda'()>(main::$_0::operator()() const::'lambda'()&&) /usr/lib/gcc/x86_
64-redhat-linux/14/../../../../include/c++/14/bits/invoke.h:96:14
    #4 0x55e21cf6b410 in decltype(auto) std::__apply_impl<main::$_0::operator()() const::'lambda'(), std::tuple<>>(main::$_0::operator()() const::'lambda'()&&, std::tuple<>&&, std::integer_sequence<unsigned long, .
..>) /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/tuple:2921:14
    #5 0x55e21cf6b3b2 in decltype(auto) std::apply<main::$_0::operator()() const::'lambda'(), std::tuple<>>(main::$_0::operator()() const::'lambda'()&&, std::tuple<>&&) /usr/lib/gcc/x86_64-redhat-linux/14/../../../
../include/c++/14/tuple:2936:14
    #6 0x55e21cf6b283 in seastar::future<int> seastar::futurize<int>::apply<main::$_0::operator()() const::'lambda'()>(main::$_0::operator()() const::'lambda'()&&, std::tuple<>&&) /home/kefu/dev/seastar/include/sea
star/core/future.hh:2005:28
    #7 0x55e21cf6b043 in seastar::futurize<std::invoke_result<main::$_0::operator()() const::'lambda'()>::type>::type seastar::async<main::$_0::operator()() const::'lambda'()>(seastar::thread_attributes, main::$_0:
:operator()() const::'lambda'()&&)::'lambda'()::operator()() const /home/kefu/dev/seastar/include/seastar/core/thread.hh:260:13
    #8 0x55e21cf6ae74 in seastar::noncopyable_function<void ()>::direct_vtable_for<seastar::futurize<std::invoke_result<main::$_0::operator()() const::'lambda'()>::type>::type seastar::async<main::$_0::operator()()
 const::'lambda'()>(seastar::thread_attributes, main::$_0::operator()() const::'lambda'()&&)::'lambda'()>::call(seastar::noncopyable_function<void ()> const*) /home/kefu/dev/seastar/include/seastar/util/noncopyable
_function.hh:129:20
    #9 0x7f5d757a0fb3 in seastar::noncopyable_function<void ()>::operator()() const /home/kefu/dev/seastar/include/seastar/util/noncopyable_function.hh:215:16
    #10 0x7f5d75ef5611 in seastar::thread_context::main() /home/kefu/dev/seastar/src/core/thread.cc:311:9
    #11 0x7f5d75ef50eb in seastar::thread_context::s_main(int, int) /home/kefu/dev/seastar/src/core/thread.cc:287:43
    #12 0x7f5d72f8a18f  (/lib64/libc.so.6+0x5a18f) (BuildId: b098f1c75a76548bb230d8f551eae07a2aeccf06)
```

so, in this change, let's hold it using a smart pointer, so we
can destroy it when it leaves the lexical scope.

Signed-off-by: Kefu Chai <[email protected]>

Closes scylladb#2224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rzarzynski rzarzynski rzarzynski left review comments

@tchaikov tchaikov tchaikov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cyx1231st @tchaikov @rzarzynski