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

[YSQL] Decouple YBIsDBCatalogVersionMode() from multi-tenancy use case #18346

Closed
1 task done
myang2021 opened this issue Jul 21, 2023 · 0 comments
Closed
1 task done
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug

Comments

@myang2021
Copy link
Contributor

myang2021 commented Jul 21, 2023

Jira Link: DB-7334

Description

The function YBIsDBCatalogVersionMode() was originally introduced for multi-tenancy use case. So when it returns true, it used to assume there are many databases. This is no longer true because we now also support per-database catalog version mode in single tenant clusters. We should go over the use of YBIsDBCatalogVersionMode() to fix any place where multi-tenancy use case is implied.

This was noticed when reviewing the following code:

    /*
     * YB mode uses local-tserver prefetching instead of relcache file.
     * TODO: either put this under a GUC variable or remove the old code
     * below.
     */
    if (IsYugaByteEnabled() &&
        *YBCGetGFlags()->ysql_catalog_preload_additional_tables &&
        !YBIsDBCatalogVersionMode())
        return false;

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@myang2021 myang2021 added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Jul 21, 2023
@myang2021 myang2021 self-assigned this Jul 21, 2023
@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue labels Jul 21, 2023
@myang2021 myang2021 added kind/bug This issue is a bug and removed kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue status/awaiting-triage Issue awaiting triage labels Jul 21, 2023
myang2021 added a commit that referenced this issue Jul 22, 2023
… use case

Summary:
The function `YBIsDBCatalogVersionMode()` was originally introduced for
multi-tenancy use case. So when it returns true, it used to assume that there
are many databases. This is no longer true because we now also support
per-database catalog version mode in single tenant clusters. We should
go over the use of `YBIsDBCatalogVersionMode()` to fix any place where
multi-tenancy use case is implied.

I found one such place: when `YBIsDBCatalogVersionMode()` returns true,
currently we will not use tserver response cache and will use rel cache init
file. The assumption was that in multi-tenancy use case there will be many
databases so the tserver response cache hit ratio will be low because different
databases cannot share the same tserver cache entry. So even when tserver
response cache is turned on by default, we should not use it for multi-tenant
clusters. Now that `YBIsDBCatalogVersionMode()` can be used in single-tenant
clusters, which typically have only a few databases, then we should not prevent
the use of tserver response cache when `YBIsDBCatalogVersionMode()` is true. To
disable tserver cache when there are many databases, users can manually set
`--ysql_enable_read_request_caching` to false.
Jira: DB-7334

Test Plan: ./yb_build.sh release --cxx-test pg_catalog_version-test

Reviewers: jason

Reviewed By: jason

Subscribers: ssong, yql

Differential Revision: https://phorge.dev.yugabyte.com/D27148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug
Projects
None yet
Development

No branches or pull requests

2 participants