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

WIP: Consistent hashing for balancer #258

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

WIP: Consistent hashing for balancer #258

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
70cbd79
wlc mowed to from dataplane to controlplane
levon-a-akopian Jun 26, 2024
59e9b3e
Refactors service evaluation
Nov 14, 2024
b98de20
Adds chash service evaluation stub
Nov 14, 2024
95e01ce
Adds handling of chash services to dataplane
Nov 18, 2024
e5ae4d4
Works. Maybe
Dec 3, 2024
bfa8e0f
Adds chash submodule
Dec 3, 2024
69fbfa9
Updates chash
Dec 3, 2024
cb856e7
Fixes build
Dec 3, 2024
ad4c864
Updates chash
Dec 3, 2024
7d8b42d
Changes chash submodule to track master
Dec 4, 2024
c07d5a7
Update chash
Dec 4, 2024
f613799
Disables unittests in chash subproject
Dec 4, 2024
61a6aa8
Fixes build
Dec 4, 2024
3b6dfc7
Fixes ubuntu 18 includes
Dec 5, 2024
df6aeb4
Updates chash submodule
Dec 5, 2024
0cf7590
Switches default meson option to link statically
Dec 5, 2024
7089d5a
No config params
Dec 9, 2024
4f3e395
Updates chash fixing edge cases
Dec 12, 2024
5122d78
Fixes wlc using chash
Dec 12, 2024
a5ee74f
Adds feedback on autotests errors
Dec 12, 2024
7259efa
Updates autotest 055_balancer_wlc expected data
Dec 12, 2024
cd20fd0
Replaces goto with deferer object
Dec 6, 2024
ed5a090
Changes autotest
Dec 17, 2024
db3d789
Constants
Dec 17, 2024
20297e6
Some stylefix
Dec 17, 2024
9eb04b3
References
Dec 17, 2024
92d2254
Reserves
Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions common/deferer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
namespace utils
{

template<typename F>
class Deferer
{
F action_;

public:
Deferer(F&& action) :
action_{std::move(action)}
{}
Deferer(const Deferer& other) = delete;
Deferer(Deferer&& other) = delete;
Deferer& operator=(const Deferer& other) = delete;
Deferer& operator=(Deferer&& other) = delete;
~Deferer()
{
action_();
}
};

} // namespace utils
6 changes: 3 additions & 3 deletions dataplane/globalbase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "worker_gc.h"

#include "common/counters.h"
#include "common/deferer.h"
#include "common/define.h"

#include "debug_latch.h"
Expand Down Expand Up @@ -1612,6 +1613,7 @@ balancer_real_id_t* generation::rebuild_service_ring_one_wrr(
const balancer_real_id_t* const do_not_exceed,
const balancer_service_t* service)
{
utils::Deferer defer([]() { YADECAP_MEMORY_BARRIER_COMPILE; });
for (uint32_t real_idx = service->real_start;
Copy link
Collaborator

@ol-imorozko ol-imorozko Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

IMHO this makes code unnecessary complex. Maybe with more complex logic it will be fine, but here I feel like just doing

//...
	if (start == do_not_exceed)
	
		YANET_LOG_ERROR("Balancer service exceeded ring chunk bounds\n");
		YADECAP_MEMORY_BARRIER_COMPILE
		return start;
	}
//...
	YADECAP_MEMORY_BARRIER_COMPILE;
	return start;
}

is sufficient enough

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to YADECAP_MEMORY_BARRIER_COMPILE at each function exit path. Using it at each existing function exit doesn't convey this intent to future function changes.

How about using a DEFERED_MEMORY_BARRIER_COMPILE macro?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, maybe your solution makes more sense that I've thought initially.
smth like this will suffice?

struct MemoryBarrierGuard {
    ~MemoryBarrierGuard() {
        YADECAP_MEMORY_BARRIER_COMPILE;
    }
};

And do that at the start of every method:

Result cControlPlane::init(bool use_kernel_interface)
{
    MemoryBarrierGuard guard;
    //...

real_idx < service->real_start + service->real_size;
++real_idx)
Expand All @@ -1631,15 +1633,13 @@ balancer_real_id_t* generation::rebuild_service_ring_one_wrr(
if (start == do_not_exceed)
{
YANET_LOG_ERROR("Balancer service exceeded ring chunk bounds\n");
goto finish;
return start;
}
*start = real_id;
++start;
}
}

finish:
YADECAP_MEMORY_BARRIER_COMPILE;
return start;
}

Expand Down
Loading