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

[docdb] Master should rebuild YCQL system.partitions on a background thread #6445

Closed
bmatican opened this issue Nov 20, 2020 · 4 comments
Closed
Assignees
Labels
area/docdb YugabyteDB core features priority/high High Priority

Comments

@bmatican
Copy link
Contributor

Currently, we cache the system.partitions iterator data from src/yb/master/yql_partitions_vtable.cc in CatalogManager, but in case of changes, we recreate it on the fly, on a new incoming request. This lazy approach puts the onus on incoming requests. Moreover, if many changes are happening in the cluster, this leads to a lot of work for the master, to keep on re-computing this. Finally, the client might not necessarily need the most up to date copy of the data! It could already have stale data, as TS state changing is reported to the master via heartbeats, so it's not an 100% accurate reflection of reality...

We could move the computation of this to either a new background thread, or the existing one in src/yb/master/catalog_manager_bg_tasks.cc, and recompute the content in case of changes, much like now, but at a lower frequency. We could add a new gflag for the frequency and just ensure that, if more than that time has passed, we recompute. Meanwhile, the partitions table can reply with the existing stale data.

cc @nspiegelberg @kmuthukk

@bmatican
Copy link
Contributor Author

bmatican commented Dec 17, 2020

Part of #6664 #6690

hulien22 added a commit that referenced this issue Dec 17, 2020
…ground thread

Summary:
Adding a new gflag `partitions_vtable_cache_refresh_secs` to control this. When this flag is set
then any calls to `YQLPartitionsVTable::RetrieveData` will just immediately return the cached
values. A new bg task (via `background_tasks_thread_pool_`) will now be in charge of rebuilding the
cache, using the new gflag to determine how long to wait between rebuilds. If the flag is set to 0,
then our old behaviour will be used (ie the cache will be rebuilt on incoming requests and not in a
background thread).

Test Plan:
```
ybd --gtest_filter CppCassandraDriverTest.YQLPartitionsVtableCacheRefresh
```

Reviewers: rahuldesirazu, sergei, nicolas

Reviewed By: nicolas

Subscribers: zyu, ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D9999
@bmatican
Copy link
Contributor Author

We had a follow up diff to fix a shutdown related issue in 8c98f6c

@bmatican
Copy link
Contributor Author

bmatican commented Feb 5, 2021

This fix is also going to be necessary: #7109

nspiegelberg pushed a commit that referenced this issue Feb 24, 2021
…tions on a background thread

Summary:
Adding a new gflag `partitions_vtable_cache_refresh_secs` to control this. When this flag is set
then any calls to `YQLPartitionsVTable::RetrieveData` will just immediately return the cached
values. A new bg task (via `background_tasks_thread_pool_`) will now be in charge of rebuilding the
cache, using the new gflag to determine how long to wait between rebuilds. If the flag is set to 0,
then our old behaviour will be used (ie the cache will be rebuilt on incoming requests and not in a
background thread).

Original commit:  D9999 / 71ed762

Test Plan:
Jenkins: rebase: 2.4, hot

```
ybd --gtest_filter CppCassandraDriverTest.YQLPartitionsVtableCacheRefresh
```

Reviewers: rahuldesirazu, sergei, bogdan

Reviewed By: bogdan

Subscribers: dsrinivasan, ybase, zyu

Differential Revision: https://phabricator.dev.yugabyte.com/D10695
@bmatican
Copy link
Contributor Author

#6902 was also a followup fix to 8c98f6c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docdb YugabyteDB core features priority/high High Priority
Projects
None yet
Development

No branches or pull requests

2 participants