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.

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

Optimize Shard.list and Shard.get_by_key #127

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

Conversation

indrekj
Copy link
Contributor

@indrekj indrekj commented Jul 15, 2019

Previous list and get_by_key had to go through GenServer to acquire
values ets table and replicas information. In case GenServer was
processing an update (e.g. heartbeat, track, untrack) then list and
get_by_key functions were blocked until it was completed. We saw this
behaviour in our cluster where simple list/get_by_key calls were
sometimes taking over few hundred milliseconds.

Storing replicas information in an ets table allows us to avoid going
through genserver and allows us to process list/get_by_key immediately.

I removed dirty_list function which was not public / exposed and which
was trying to resolve the same issue. dirty_list was called dirty
because it didn't check for down_replicas. This solution checks
down_replicas and doesn't change the api interface.

Update 2019/12/06: We've fully rolled this out to production (50K+ concurrent connections). We also got ~30% drop in CPU usage which I did not expect at all, but that's very good.

Update 2020/01/03: We've hit 70K+ concurrent connections. Everything still looking good.

Update 2021/06/13: Over 200K concurrent connections with this.

This should also resolve #124

@indrekj indrekj force-pushed the fast-list branch 2 times, most recently from e435ace to 57d43f9 Compare July 18, 2019 22:16
Previous list and get_by_key had to go through GenServer to acquire
values ets table and replicas information. In case GenServer was
processing an update (e.g. heartbeat, track, untrack) then list and
get_by_key functions were blocked until it was completed. We saw this
behaviour in our cluster where simple list/get_by_key calls were
sometimes taking over few hundred milliseconds.

Storing down replicas information in an ets table allows us to avoid
going through genserver and allows us to process list/get_by_key
immediately.

I removed dirty_list function which was not public / exposed and which
was trying to resolve the same issue. dirty_list was called dirty
because it didn't check for down_replicas. This solution checks
down_replicas and doesn't change the api interface.

This should also resolve phoenixframework#124
@indrekj
Copy link
Contributor Author

indrekj commented Aug 3, 2019

Tried this out in production (patched v1.1.2). Forwarded ~15% traffic to the patched instance. Given endpoint does 1-40 get_by_key calls depending on the input. Here are the results:
Screen Shot 2019-08-03 at 11 33 59
First two graphs are from the v1.1.2 instances and the last one is from the patched one.

It looks faster, I'd say 20-50% faster. I was actually expecting better results but still better than before.

@indrekj
Copy link
Contributor Author

indrekj commented Oct 28, 2019

@chrismccord any chance of getting this reviewed / merged as well?

https://erlang.org/doc/man/ets.html

> Performance tuning. Defaults to false. When set to true, the table is
optimized for concurrent read operations. When this option is enabled on
a runtime system with SMP support, read operations become much cheaper;
especially on systems with multiple physical processors. However,
switching between read and write operations becomes more expensive.

We have a lot of read operations. Each time get_by_key or get_by_topic
is called then this table is read. We want it to be as fast as possible.

> You typically want to enable this option when concurrent read
operations are much more frequent than write operations, or when
concurrent reads and writes comes in large read and write bursts (that
is, many reads not interrupted by writes, and many writes not
interrupted by reads).

This table is rarely updated. It is only updated when a replica goes
down or when that replica comes back up.
@luislhsc
Copy link

Any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Providing custom Tracker.list timeout
2 participants