[docdb] Re-enable yql partitions vtable cache improvements by default #8978
Comments
@bmatican, we still have this test: |
…of via a background task Summary: Changing the way that we generate the system.partitions table. Before, we would regenerate the entire table on a background thread in case there were any changes to the catalog manager's tablet map / table ids map / tablet locations version. This diff changes this to update only necessary portions of the vtable, and only when there are relevant changes that occur. This is done by updating the table on table drops and on tablet reports. In order to ensure that we still maintain order (of table ids and partitions), and that we are still efficient, this diff introduces a map: ``` map<TableId, map<string, QLRow>> table_to_partition_start_to_row_map_ ``` This map holds the true state of the vtable, and on requests, we convert this map into the appropriate `QLRowBlock` object. This object is also cached in order to reduce how often we need to recreate it. Adding in a few new flags to control these additions: `generate_partitions_vtable_on_changes` - when enabled this will enable this feature `catalog_manager_check_yql_partitions_exist_for_is_create_table_done` - when enabled, we will wait for all of a table's partitions to appear in system.partitions during a CreateTable request Test Plan: `ybd --gtest_filter CppCassandraDriverTest.YQLPartitionsVtableCacheUpdateOnChanges` Also modified `ybd --gtest_filter CppCassandraDriverTest.YQLPartitionsVtableCacheRefresh` to test the old bg task flow. Reviewers: bogdan, sergei Reviewed By: sergei Subscribers: zyu, ybase Differential Revision: https://phabricator.dev.yugabyte.com/D12513
hulien22
changed the title
yugabyte-ci
added
kind/bug
|
Adding some additional notes on this: Originally had a design to invalidate the (entire) cache on table creation, so that new tables would always be seen in the cache. But after some discussions, decided that we wanted a more robust fix that would only invalidate/update the out of date parts of the cache, and then remove the need for the bg thread. This is what led to https://phabricator.dev.yugabyte.com/D12513, where we update an in-memory map of the partitions on tablet reports/drop tables with the info from the new tablets only. Then on requests, if this map has been modified, the actual vtable is constructed from the map and cached for future requests. But after talking with Sergei about this, we wanted to introduce a more robust fix that would only invalidate/update the out of date parts of the cache, and then remove the need for the bg thread. This is what led to 7f65b9d, where we update an in-memory map of the partitions on tablet reports/drop tables with the info from the new tablets only. Then on requests, if this map has been modified, the actual vtable is constructed from the map and cached for future requests. |
Jira Link: DB-2682
We originally added in a bg thread to generate system.partitions (#6445) in order to reduce pressure on the master when there are a large number of connections.
We disabled
partitions_vtable_cache_refresh_secsby default in master a while back (#7284), as we were seeing perf related issues, shortly after a table is created. Essentially, tablets for new tables weren't immediately visible in the vtable (due to the bg thread not running yet) and so requests would have to perform extra network hops in order to find these tablets.First, maybe let's add a test first, to confirm if, right after a create table, a YCQL driver querying
system.partitionswill not see this table's partitions.Then, let's figure out a good way to solve for this. Maybe even a cheap solution, like invalidating the cache (and maybe quickly queueing the bg task), on a create table, might be good enough to start with. cc @rahuldesirazu @nspiegelberg
The text was updated successfully, but these errors were encountered: