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] yb_read_from_followers not used when set in function #20482

Open
1 task done
FranckPachot opened this issue Jan 5, 2024 · 2 comments
Open
1 task done

[YSQL] yb_read_from_followers not used when set in function #20482

FranckPachot opened this issue Jan 5, 2024 · 2 comments
Assignees
Labels

Comments

@FranckPachot
Copy link
Contributor

FranckPachot commented Jan 5, 2024

Jira Link: DB-9486

Description

It is possible to use follower reads for a specific query by using a function that sets the required parameters. However, it seems that yb_read_from_followers is not utilized.

Example:

-- install YBWR to get per-tablet metrics
\! curl -s https://raw.githubusercontent.com/FranckPachot/ybdemo/main/docker/yb-lab/client/ybwr.sql | grep -v '\watch' > ybwr.sql
\i ybwr.sql

-- create a table (on a RF3 cluster to have followers)
create table demo as select * from generate_series(1,6);

-- create a function to read the table and show the parameters
create or replace function f() returns table (transaction_read_only text, yb_read_from_followers text) as $$
 select current_setting('transaction_read_only'), current_setting('yb_read_from_followers') from demo;
$$ language sql;
alter function f set transaction_read_only=on;
alter function f set yb_read_from_followers=on;

-- call the function and show where it is read from
execute snap_reset;
select * from f() ;
execute snap_table;

-- set follower reads for the whole session and run the same
set yb_read_from_followers=on;
execute snap_reset;
select * from f() ;
execute snap_table;

The first run shows that yb_read_from_followers is on but reads only from the leaders:

yugabyte=# execute snap_reset;
 ybwr metrics
--------------
(0 rows)

yugabyte=# select * from f() ;
 transaction_read_only | yb_read_from_followers
-----------------------+------------------------
 on                    | on
 on                    | on
 on                    | on
 on                    | on
 on                    | on
 on                    | on
(6 rows)

yugabyte=# execute snap_table;
 I-seek | I-next | I-prev | R-seek | R-next | R-prev | insert | dbname relname tablet (L)eader node | (I)ntents (R)regular
--------+--------+--------+--------+--------+--------+--------+------------------------------------------------------------
        |        |        |      1 |        |        |        | yugabyte demo hash_split: [0x0000, 0x5554] L 10.0.0.40
        |        |        |      1 |      2 |        |        | yugabyte demo hash_split: [0x5555, 0xAAA9] L 10.0.0.39
        |        |        |      1 |      4 |        |        | yugabyte demo hash_split: [0xAAAA, 0xFFFF] L 10.0.0.41
(3 rows)

The second run set yb_read_from_followers at session level and reads from followers in the same node (10.0.0.39):

yugabyte=# set yb_read_from_followers=on;
SET
yugabyte=# execute snap_table;
 I-seek | I-next | I-prev | R-seek | R-next | R-prev | insert | dbname relname tablet (L)eader node | (I)ntents (R)regular
--------+--------+--------+--------+--------+--------+--------+------------------------------------------------------------
(0 rows)

yugabyte=# select * from f() ;
 transaction_read_only | yb_read_from_followers
-----------------------+------------------------
 on                    | on
 on                    | on
 on                    | on
 on                    | on
 on                    | on
 on                    | on
(6 rows)

yugabyte=# execute snap_table;
 I-seek | I-next | I-prev | R-seek | R-next | R-prev | insert | dbname relname tablet (L)eader node | (I)ntents (R)regular
--------+--------+--------+--------+--------+--------+--------+------------------------------------------------------------
        |        |        |      1 |        |        |        | yugabyte demo hash_split: [0x0000, 0x5554]   10.0.0.39
        |        |        |      1 |      2 |        |        | yugabyte demo hash_split: [0x5555, 0xAAA9] L 10.0.0.39
        |        |        |      1 |      4 |        |        | yugabyte demo hash_split: [0xAAAA, 0xFFFF]   10.0.0.39
(3 rows)

A workaround is to set yb_read_from_followers before the transaction starts but then all read only transactions will do stale reads, which is not desirable.

Issue Type

kind/bug

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

  • I confirm this issue does not contain any sensitive information.
@FranckPachot FranckPachot added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Jan 5, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jan 5, 2024
@FranckPachot
Copy link
Contributor Author

Adding that the same behavior is observed when yb_read_from_followers is set within the transaction:

yugabyte=# execute snap_reset;
 ybwr metrics
--------------
(0 rows)

yugabyte=# set yb_read_from_followers=off;
SET
yugabyte=# begin transaction;
BEGIN
yugabyte=*#  set local yb_read_from_followers=on;
SET
yugabyte=*#  select * from f() ;
 transaction_read_only | yb_read_from_followers
-----------------------+------------------------
 on                    | on
 on                    | on
 on                    | on
 on                    | on
 on                    | on
 on                    | on
(6 rows)

yugabyte=*#  commit;
COMMIT
yugabyte=# execute snap_table;
 I-seek | I-next | I-prev | R-seek | R-next | R-prev | insert | dbname relname tablet (L)eader node | (I)ntents (R)regular
--------+--------+--------+--------+--------+--------+--------+------------------------------------------------------------
        |        |        |      1 |      3 |        |        | yugabyte demo hash_split: [0x0000, 0x5554] L 10.0.0.40
        |        |        |      1 |      3 |        |        | yugabyte demo hash_split: [0x5555, 0xAAA9] L 10.0.0.41
        |        |        |      1 |        |        |        | yugabyte demo hash_split: [0xAAAA, 0xFFFF] L 10.0.0.39
(3 rows)

@rthallamko3 rthallamko3 removed the status/awaiting-triage Issue awaiting triage label Jan 17, 2024
amitanandaiyer added a commit that referenced this issue Jul 15, 2024
Summary:
YugabyteDB has 2 GUCs to control follower reads:
* `yb_read_from_followers`: this enables follower reads on READ ONLY transaction blocks that have started after the `yb_read_from_followers` GUC was turned on. A transaction block is anything starting with a BEGIN statement until a COMMIT/ROLLBACK statement.
* `yb_follower_read_staleness_ms`: This controls the staleness of follower reads i.e,. for READ ONLY transaction blocks after `yb_read_from_followers` is set.

Prior to this change, the 2 follower reads GUCs have a behaviour similar to the `default_transaction_*` GUC. They apply to all read only transaction blocks that have started after the GUC is set. If the GUC is set inside a transaction block, there is no effect on that specific transaction.

With this change,
  * If the "follower read" GUCs are set after a BEGIN statement but before any actual query is executed, we want the GUCs to take effect on that transaction too. This is similar to the other `transaction_*` GUCs.
 * If an actual query has already been executed in the transaction block, an error will be returned to the user similar to case (1) shown above for the `transaction_*` GUCs

Additionally, another minor change is the limit checking for `yb_follower_read_staleness_ms`. For safety reasons `yb_follower_read_staleness_ms` is not allowed to be set to something lower than 2 * `max_clock_skew`. Before this diff, this check was enforced only if follower reads was enabled. However with this diff, we enforce the check on `yb_follower_read_staleness_ms` regardless of whether follower reads is enabled.

**Backwards compatibility:**

There could be customers who were setting follower reads GUCs after executing some actual queries in the transaction block. If they are caught by surprise on receiving an error and want to switch to the older behaviour until they fix their app logic, they can set a Tserver gflag called:

`ysql_follower_reads_behavior_before_fixing_20482` to preserve old behavior.
This can either be updated by setting the Gflag on the TServer, or by setting the GUC variable `follower_reads_behavior_before_fixing_20482` in the ysql session.
Jira: DB-9486

Test Plan:
yb_build.sh --cxx-test 'TEST_F(PgMiniTest, FollowerReads) {' --test_args --vmodule=libpq_utils=2
yb_build.sh --java-test 'org.yb.pgsql.TestPgFollowerReads#testSetIsolationLevelsWithReadFromFollowersSessionVariable'

Reviewers: pjain, rthallam

Reviewed By: pjain

Subscribers: rthallam, yql

Differential Revision: https://phorge.dev.yugabyte.com/D31540
jasonyb pushed a commit that referenced this issue Jul 17, 2024
Summary:
 de8c392 [#22923] YSQL: In EXPLAIN command, fix relids of prefix keys when under a subquery distinct index scan.
 a137121 [#22349] yugabyted: xCluster Integration with yugabyted.
 040071d note about Jumpcloud support (#23193)
 d1572d9 [PLAT-13410] Allow YCQL index table selection
 Excluded: 47f40bb [#22144, #22145] YSQL, QueryDiagnostics: Infrastructure for QueryDiagnostics
 c51e6c9 [PLAT-14519] Master failover must not fail when some tservers are down
 c319a56 [#23100] YSQL: drop column does not complete in docdb when alter schema rpc is missed
 26eaeaa [#23071] yugabyted: Add tests for yugabyted cluster creation.
 160f24e [PLAT-14162] Support bootstrapping for editing list of databases in DR replicatoin
 a65795e [doc][yba] Ports and encryption (#23152)
 2ca9deb [#23079] DocDB: Small cleanups to stay closer to pg15-upgrade branch
 cba0254 [PLAT-14655] Fix ephemeral upload directory on container installs
 6e99ea0 [PLAT-14546] Ansible task to replace CentOS 7 repo URLs
 a9f675e [PLAT-14570] Deploy YBC build based on the architecture of destination node
 d56446f [docs] Updated OS tables (#23156)
 019ab00 [#23055] docdb: Add in-progress restore/deletion checks to DeleteSnapshot and RestoreSnapshot.
 9aed20a [PLAT-14248][PLAT-14249] - feat : Enable/Disable Enhanced PG Compatibility
 79888cf [#20482] YSQL : Allow updating follower reads in a transaction block
 a946721 [#23122] YSQL: Add pg_partman to build
 e5aea11 [PLAT-11188] Handle a node instance without node name set properly in health checks
 04cf65d [#13549] YSQL: Disable ACCESS EXCLUSIVE LOCK in pg_partman
 ed1809c [#23123] YSQL: Disable other partition methods except native in pg_partman
 bc55a46 [#23125] YSQL: Disable pg_partman background worker
 e1ef208 [#23124] YSQL: Disable advisory locks usage in pg_partman

- explain.c:
  - yb_fix_indexpr_mutator: master
    de8c392 adds this function.  Change
    varnoold to varnosyn in accordance with upstream PG
    9ce77d75c5ab094637cc4a446296dc3be6e3c221.

Test Plan: Jenkins: rebase: pg15-cherrypicks, urgent

Reviewers: jason, tfoucher

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D36576
@jameshartig
Copy link
Contributor

I believe the fix for this caused a regression filed here: #25635.

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

No branches or pull requests

5 participants