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

Reduce unncessary CACHE_HITRATES updates in gossip #5971

Closed
asias opened this issue Mar 6, 2020 · 8 comments
Closed

Reduce unncessary CACHE_HITRATES updates in gossip #5971

asias opened this issue Mar 6, 2020 · 8 comments
Labels
area/gossip area/stability feature/enhancement symptom/performance Issues causing performance problems
Milestone
5.2

Comments

@asias
Copy link
Contributor

asias commented Mar 6, 2020

The size of CACHE_HITRATES message is O(n), n is the number of tables. The message can be very big.

We update CACHE_HITRATES unconditionally and periodically even if the values are not changed. I am wondering if we could avoid sending CACHE_HITRATES if it does not change since last update.

INFO  2020-03-06 17:13:50,054 [shard 0] storage_service - Update system.peers table: endpoint=127.0.0.3, app_state=CACHE_HITRATES, versioned_value=Value(system_distributed.view_build_status:0.000000;system_auth.role_members:0.00000
0;system_distributed.cdc_topology_description:0.153518;system_traces.sessions_time_idx:0.000000;system_auth.roles:1.000000;system_traces.node_slow_log_time_idx:0.000000;system_traces.sessions:0.000000;myks2.standard1:0.000000;syste
m_distributed.cdc_description:0.000000;system_traces.node_slow_log:0.000000;system_traces.events:0.000000;,732)

INFO  2020-03-06 17:13:52,055 [shard 0] storage_service - Update system.peers table: endpoint=127.0.0.3, app_state=CACHE_HITRATES, versioned_value=Value(system_distributed.view_build_status:0.000000;system_auth.role_members:0.00000
0;system_distributed.cdc_topology_description:0.153518;system_traces.sessions_time_idx:0.000000;system_auth.roles:1.000000;system_traces.node_slow_log_time_idx:0.000000;system_traces.sessions:0.000000;myks2.standard1:0.000000;syste
m_distributed.cdc_description:0.000000;system_traces.node_slow_log:0.000000;system_traces.events:0.000000;,740)   

INFO  2020-03-06 17:13:54,062 [shard 0] storage_service - Update system.peers table: endpoint=127.0.0.2, app_state=CACHE_HITRATES, versioned_value=Value(system_distributed.view_build_status:0.000000;system_auth.role_members:0.00000
0;system_distributed.cdc_topology_description:0.500000;system_traces.sessions_time_idx:0.000000;system_auth.roles:1.000000;system_traces.node_slow_log_time_idx:0.000000;system_traces.sessions:0.000000;myks2.standard1:0.000000;syste
m_distributed.cdc_description:0.500000;system_traces.node_slow_log:0.000000;system_traces.events:0.000000;,772)

The downside of such updates:

- Introduces more gossip exchange traffic

- Updates system.peers all the time
@asias
Copy link
Contributor Author

asias commented Mar 6, 2020

@gleb-cloudius What do you think?

@slivne slivne added this to the 3.x milestone Mar 17, 2020
@slivne
Copy link
Contributor

slivne commented Mar 17, 2020

we had

#5200

and AFAIK @haaawk even coded a suggestion related to this that was not merged

@haaawk
Copy link
Contributor

haaawk commented Mar 17, 2020

POC is here https://github.com/haaawk/scylla/commits/5200-v1

@asias
Copy link
Contributor Author

asias commented Sep 25, 2020

POC is here https://github.com/haaawk/scylla/commits/5200-v1

The link is dead. I want to revive this work. The extra unnecessary traffic is fine to a cluster in a good shape but when some of the node or shards are loaded, such messages and the handling of such messages can make the system even busy.

@haaawk haaawk assigned haaawk and StarostaGit and unassigned haaawk Oct 11, 2020
@asias
Copy link
Contributor Author

asias commented Oct 13, 2020

Ping. It is not urgent.

OK, I saw you assigned someone.

@haaawk haaawk removed their assignment Feb 3, 2021
@haaawk haaawk assigned haaawk and unassigned StarostaGit Mar 21, 2021
@bhalevy
Copy link
Member

bhalevy commented Jul 5, 2022

@asias can you pick this up?

@denesb
Copy link
Contributor

denesb commented Jul 5, 2022

Can we use table ids in the state string, instead of table names? Table names can be arbitrary long, whereas id as UUIDs with a fixed size.

@bhalevy bhalevy mentioned this issue Jul 19, 2022
Closed
asias added a commit to asias/scylla that referenced this issue Jul 20, 2022
@asias
misc_services: Fix cache hitrate update
5898893 
This patch avoids unncessary CACHE_HITRATES updates through gossip.

After this patch:

Publish CACHE_HITRATES in case:

- We haven't published it at all
- The diff is bigger enough and we haven't published in the last 5 seconds

Note: A peer node can know the cache hitrate through read_data
read_mutation_data and read_digest RPC verbs which have cache_temperature in
the response. So there is no need to update CACHE_HITRATES through gossip in
high frequency.

We do the recalculation faster if the diff is bigger than 0.01. It is useful to
do the calculation even if we do not publish the CACHE_HITRATES though gossip,
since the recalculation will call the table->set_global_cache_hit_rate to set
the hitrate.

Fixes scylladb#5971
asias added a commit to asias/scylla that referenced this issue Jul 20, 2022
@asias
misc_services: Fix cache hitrate update
b399de0 
This patch avoids unncessary CACHE_HITRATES updates through gossip.

After this patch:

Publish CACHE_HITRATES in case:

- We haven't published it at all
- The diff is bigger enough and we haven't published in the last 5 seconds

Note: A peer node can know the cache hitrate through read_data
read_mutation_data and read_digest RPC verbs which have cache_temperature in
the response. So there is no need to update CACHE_HITRATES through gossip in
high frequency.

We do the recalculation faster if the diff is bigger than 0.01. It is useful to
do the calculation even if we do not publish the CACHE_HITRATES though gossip,
since the recalculation will call the table->set_global_cache_hit_rate to set
the hitrate.

Fixes scylladb#5971
@asias asias mentioned this issue Jul 20, 2022
Closed
@asias
Copy link
Contributor Author

asias commented Jul 20, 2022

I've sent an PR here: #11079

asias added a commit to asias/scylla that referenced this issue Jul 20, 2022
@asias
misc_services: Fix cache hitrate update
352ebeb 
This patch avoids unncessary CACHE_HITRATES updates through gossip.

After this patch:

Publish CACHE_HITRATES in case:

- We haven't published it at all
- The diff is bigger than 1% and we haven't published in the last 5 seconds
- The diff is really big 10%

Note: A peer node can know the cache hitrate through read_data
read_mutation_data and read_digest RPC verbs which have cache_temperature in
the response. So there is no need to update CACHE_HITRATES through gossip in
high frequency.

We do the recalculation faster if the diff is bigger than 0.01. It is useful to
do the calculation even if we do not publish the CACHE_HITRATES though gossip,
since the recalculation will call the table->set_global_cache_hit_rate to set
the hitrate.

Fixes scylladb#5971
@DoronArazii DoronArazii modified the milestones: 5.x, 5.2 Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/gossip area/stability feature/enhancement symptom/performance Issues causing performance problems
Projects
None yet
Milestone
5.2
Development

Successfully merging a pull request may close this issue.

misc_services: Fix cache hitrate update asias/scylla
7 participants
@asias @haaawk @denesb @slivne @bhalevy @StarostaGit @DoronArazii