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

Support Kubernetes style cname in YugaByteDB clients #23

Closed
rkarthik007 opened this issue Jan 26, 2018 · 2 comments
Closed

Support Kubernetes style cname in YugaByteDB clients #23

rkarthik007 opened this issue Jan 26, 2018 · 2 comments
Assignees
@ttyusupov
Labels
kind/enhancement This is an enhancement of an existing feature
Milestone
v1.0

Comments

@rkarthik007
Copy link
Collaborator

Create a way for the Cassandra and Redis clients to work with Kubernetes style cnames when a YB cluster is deployed using k8s.
@mbautin
Copy link
Collaborator

mbautin commented Jan 31, 2018

The first step is to test what the Cassandra client will do by default if the server host name is a host name that resolves to multiple IP addresses.

@rkarthik007 rkarthik007 added the kind/enhancement This is an enhancement of an existing feature label Apr 6, 2018
@rkarthik007
Copy link
Collaborator Author

Update from Timur:

Cassandra java driver

It will use all addresses resolved: https://github.com/datastax/java-driver/blob/57724bd63c3038e3728260c7640abcea06f33b86/driver-core/src/main/java/com/datastax/driver/core/Cluster.java#L885, but all should use the same port. The client app can use this API instead of resolving address by itself.

Redis java driver (Jedis)

Jedis accepts hostname/address and if we pass yb-tservers.default.svc.cluster.local it will use the first resolved address to connect to the first node and then obtain other nodes addresses from that node. Being able to connect to multiple nodes is covered in #144.

@rkarthik007 rkarthik007 added this to the v1.0 milestone Apr 11, 2018
yugabyte-ci pushed a commit that referenced this issue Apr 11, 2018
@ttyusupov
#23: Use cassandra driver's DNS resolution
d0679af 
Summary:
Instead of resolving nodes to IP addresses inside sample app, used appropariate
cassandra driver's client builder. Also restricted to only allow use the same CQL port number for
all specified `--nodes` parameters, since different ports are not supported by client builder.

Test Plan:
- Need https://kubernetes.io/docs/getting-started-guides/minikube for testing.
- Build docker image to be used for minikube:
```
minikube start
eval $(minikube docker-env)

cd ~/code/devops/docker/images/yugabyte
docker build -t yugabytedb/yugabyte:test .
```
- Run local k8s cluster using
  https://github.com/YugaByte/yugabyte-db/blob/master/cloud/kubernetes/README.md. Before running
  replace `yugabytedb/yugabyte:latest` with `yugabytedb/yugabyte:test` and set `imagePullPolicy: Never` in
  `yugabyte-statefulset.yaml`.
- kubectl exec -it yb-master-0 -- yum -y install java-1.8.0-openjdk-headless.x86_64
- kubectl exec -it yb-master-0 -- java -jar /home/yugabyte/java/yb-sample-apps.jar --num_reads 1000000 --num_threads_read 4 --num_unique_keys 1000000 --num_writes 10000000 --num_threads_write 12 --workload CassandraKeyValue --nodes yb-tservers.default.svc.cluster.local:9042
It should log line similar to and list 3 IPs:
```
2018-04-10 17:31:53,814 [INFO|com.yugabyte.sample.apps.AppBase|AppBase] Connecting to nodes: yb-tservers.default.svc.cluster.local/172.17.0.7:9042,yb-tservers.default.svc.cluster.local/172.17.0.8:9042,yb-tservers.default.svc.cluster.local/172.17.0.9:9042
```
- Check the same with `--workload CassandraHelloWorld`

Reviewers: mihnea, karthik

Reviewed By: karthik

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D4584
@bbaddepudi bbaddepudi mentioned this issue Sep 4, 2018
Closed
@mbautin mbautin mentioned this issue Apr 9, 2019
Closed
@hectorgcr hectorgcr mentioned this issue Apr 9, 2019
Closed
@OlegLoginov OlegLoginov mentioned this issue Apr 23, 2019
Closed
This was referenced May 6, 2019
Closed
Closed
mbautin pushed a commit that referenced this issue Jun 20, 2019
@schoudhury
Merge pull request #23 from rkarthik007/master
879c4d9 
Create v1.1 directory of the docs
@rao-vasireddy rao-vasireddy mentioned this issue Sep 18, 2019
Closed
@d-uspenskiy d-uspenskiy mentioned this issue Nov 7, 2019
Closed
@daniel-yb daniel-yb mentioned this issue Sep 1, 2020
Closed
@mbautin mbautin mentioned this issue Oct 30, 2020
Open
@mbautin mbautin mentioned this issue Dec 5, 2020
Open
@rao-vasireddy rao-vasireddy mentioned this issue Jul 1, 2021
Closed
@sushantrmishra sushantrmishra mentioned this issue May 25, 2022
Closed
@shantanugupta-yb shantanugupta-yb mentioned this issue Jun 23, 2022
Open
@rthallamko3 rthallamko3 mentioned this issue Aug 17, 2022
Closed
jasonyb pushed a commit that referenced this issue Dec 15, 2023
@jaki
[pg15] fix: YB Seq Scan bugs
de3772b 
Summary:
YB Seq Scan code path is not hit because Foreign Scan is the default and
pg hint plan does not work.  Upcoming merge with YB master will bring in
master commit 465ee2c which changes the
default to YB Seq Scan.

To test YB Seq Scan, a temporary patch is needed (see the test plan).
With that, two bugs are encountered: fix them.

1. FailedAssertion("TTS_IS_VIRTUAL(slot)"

   On simple test case

       create table t (i int primary key, j int);
       select * from t;

   get

       TRAP: FailedAssertion("TTS_IS_VIRTUAL(slot)", File: "../../../../../../../src/postgres/src/backend/access/yb_access/yb_scan.c", Line: 3473, PID: 2774450)

   Details:

       #0  0x00007fd52616eacf in raise () from /lib64/libc.so.6
       #1  0x00007fd526141ea5 in abort () from /lib64/libc.so.6
       #2  0x0000000000af33ad in ExceptionalCondition (conditionName=conditionName@entry=0xc2938d "TTS_IS_VIRTUAL(slot)", errorType=errorType@entry=0xc01498 "FailedAssertion",
           fileName=fileName@entry=0xc28f18 "../../../../../../../src/postgres/src/backend/access/yb_access/yb_scan.c", lineNumber=lineNumber@entry=3473)
           at ../../../../../../../src/postgres/src/backend/utils/error/assert.c:69
       #3  0x00000000005c26bd in ybFetchNext (handle=0x2600ffc43680, slot=slot@entry=0x2600ff6c2980, relid=16384)
           at ../../../../../../../src/postgres/src/backend/access/yb_access/yb_scan.c:3473
       #4  0x00000000007de444 in YbSeqNext (node=0x2600ff6c2778) at ../../../../../../src/postgres/src/backend/executor/nodeYbSeqscan.c:156
       #5  0x000000000078b3c6 in ExecScanFetch (node=node@entry=0x2600ff6c2778, accessMtd=accessMtd@entry=0x7de2b9 <YbSeqNext>, recheckMtd=recheckMtd@entry=0x7de26e <YbSeqRecheck>)
           at ../../../../../../src/postgres/src/backend/executor/execScan.c:133
       #6  0x000000000078b44e in ExecScan (node=0x2600ff6c2778, accessMtd=accessMtd@entry=0x7de2b9 <YbSeqNext>, recheckMtd=recheckMtd@entry=0x7de26e <YbSeqRecheck>)
           at ../../../../../../src/postgres/src/backend/executor/execScan.c:182
       #7  0x00000000007de298 in ExecYbSeqScan (pstate=<optimized out>) at ../../../../../../src/postgres/src/backend/executor/nodeYbSeqscan.c:191
       #8  0x00000000007871ef in ExecProcNodeFirst (node=0x2600ff6c2778) at ../../../../../../src/postgres/src/backend/executor/execProcnode.c:480
       #9  0x000000000077db0e in ExecProcNode (node=0x2600ff6c2778) at ../../../../../../src/postgres/src/include/executor/executor.h:285
       #10 ExecutePlan (execute_once=<optimized out>, dest=0x2600ff6b1a10, direction=<optimized out>, numberTuples=0, sendTuples=true, operation=CMD_SELECT,
           use_parallel_mode=<optimized out>, planstate=0x2600ff6c2778, estate=0x2600ff6c2128) at ../../../../../../src/postgres/src/backend/executor/execMain.c:1650
       #11 standard_ExecutorRun (queryDesc=0x2600ff675128, direction=<optimized out>, count=0, execute_once=<optimized out>)
           at ../../../../../../src/postgres/src/backend/executor/execMain.c:367
       #12 0x000000000077dbfe in ExecutorRun (queryDesc=queryDesc@entry=0x2600ff675128, direction=direction@entry=ForwardScanDirection, count=count@entry=0, execute_once=<optimized out>)
           at ../../../../../../src/postgres/src/backend/executor/execMain.c:308
       #13 0x0000000000982617 in PortalRunSelect (portal=portal@entry=0x2600ff90e128, forward=forward@entry=true, count=0, count@entry=9223372036854775807, dest=dest@entry=0x2600ff6b1a10)
           at ../../../../../../src/postgres/src/backend/tcop/pquery.c:954
       #14 0x000000000098433c in PortalRun (portal=portal@entry=0x2600ff90e128, count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=true, run_once=run_once@entry=true,
           dest=dest@entry=0x2600ff6b1a10, altdest=altdest@entry=0x2600ff6b1a10, qc=0x7fffc14a13c0) at ../../../../../../src/postgres/src/backend/tcop/pquery.c:786
       #15 0x000000000097e65b in exec_simple_query (query_string=0x2600ffdc6128 "select * from t;") at ../../../../../../src/postgres/src/backend/tcop/postgres.c:1321
       #16 yb_exec_simple_query_impl (query_string=query_string@entry=0x2600ffdc6128) at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5060
       #17 0x000000000097b7a5 in yb_exec_query_wrapper_one_attempt (exec_context=exec_context@entry=0x2600ffdc6000, restart_data=restart_data@entry=0x7fffc14a1640,
           functor=functor@entry=0x97e033 <yb_exec_simple_query_impl>, functor_context=functor_context@entry=0x2600ffdc6128, attempt=attempt@entry=0, retry=retry@entry=0x7fffc14a15ff)
           at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5028
       #18 0x000000000097d077 in yb_exec_query_wrapper (exec_context=exec_context@entry=0x2600ffdc6000, restart_data=restart_data@entry=0x7fffc14a1640,
           functor=functor@entry=0x97e033 <yb_exec_simple_query_impl>, functor_context=functor_context@entry=0x2600ffdc6128)
           at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5052
       #19 0x000000000097d0ca in yb_exec_simple_query (query_string=query_string@entry=0x2600ffdc6128 "select * from t;", exec_context=exec_context@entry=0x2600ffdc6000)
           at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5075
       #20 0x000000000097fe8a in PostgresMain (dbname=<optimized out>, username=<optimized out>) at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5794
       #21 0x00000000008c8354 in BackendRun (port=0x2600ff8423c0) at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4791
       #22 BackendStartup (port=0x2600ff8423c0) at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4491
       #23 ServerLoop () at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1878
       #24 0x00000000008caa55 in PostmasterMain (argc=argc@entry=25, argv=argv@entry=0x2600ffdc01a0) at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1533
       #25 0x0000000000804ba8 in PostgresServerProcessMain (argc=25, argv=0x2600ffdc01a0) at ../../../../../../src/postgres/src/backend/main/main.c:208
       #26 0x0000000000804bc8 in main ()

       3469    ybFetchNext(YBCPgStatement handle,
       3470                            TupleTableSlot *slot, Oid relid)
       3471    {
       3472            Assert(slot != NULL);
       3473            Assert(TTS_IS_VIRTUAL(slot));

       (gdb) p *slot
       $2 = {type = T_TupleTableSlot, tts_flags = 18, tts_nvalid = 0, tts_ops = 0xeaf5e0 <TTSOpsHeapTuple>, tts_tupleDescriptor = 0x2600ff6416c0, tts_values = 0x2600ff6c2a00, tts_isnull = 0x2600ff6c2a10, tts_mcxt = 0x2600ff6c2000, tts_tid = {ip_blkid = {bi_hi = 0, bi_lo = 0}, ip_posid = 0, yb_item = {ybctid = 0}}, tts_tableOid = 0, tts_yb_insert_oid = 0}

   Fix by making YB Seq Scan always use virtual slot.  This is similar
   to what is done for YB Foreign Scan.

2. segfault in ending scan

   Same simple test case gives segfault at a later stage.

   Details:

       #0  0x00000000007de762 in table_endscan (scan=0x3debfe3ab88) at ../../../../../../src/postgres/src/include/access/tableam.h:997
       #1  ExecEndYbSeqScan (node=node@entry=0x3debfe3a778) at ../../../../../../src/postgres/src/backend/executor/nodeYbSeqscan.c:298
       #2  0x0000000000787a75 in ExecEndNode (node=0x3debfe3a778) at ../../../../../../src/postgres/src/backend/executor/execProcnode.c:649
       #3  0x000000000077ffaf in ExecEndPlan (estate=0x3debfe3a128, planstate=<optimized out>) at ../../../../../../src/postgres/src/backend/executor/execMain.c:1489
       #4  standard_ExecutorEnd (queryDesc=0x2582fdc88928) at ../../../../../../src/postgres/src/backend/executor/execMain.c:503
       #5  0x00000000007800f8 in ExecutorEnd (queryDesc=queryDesc@entry=0x2582fdc88928) at ../../../../../../src/postgres/src/backend/executor/execMain.c:474
       #6  0x00000000006f140c in PortalCleanup (portal=0x2582ff900128) at ../../../../../../src/postgres/src/backend/commands/portalcmds.c:305
       #7  0x0000000000b3c36a in PortalDrop (portal=portal@entry=0x2582ff900128, isTopCommit=isTopCommit@entry=false)
           at ../../../../../../../src/postgres/src/backend/utils/mmgr/portalmem.c:514
       #8  0x000000000097e667 in exec_simple_query (query_string=0x2582ffdc6128 "select * from t;") at ../../../../../../src/postgres/src/backend/tcop/postgres.c:1331
       #9  yb_exec_simple_query_impl (query_string=query_string@entry=0x2582ffdc6128) at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5060
       #10 0x000000000097b79a in yb_exec_query_wrapper_one_attempt (exec_context=exec_context@entry=0x2582ffdc6000, restart_data=restart_data@entry=0x7ffc81c0e7d0,
           functor=functor@entry=0x97e028 <yb_exec_simple_query_impl>, functor_context=functor_context@entry=0x2582ffdc6128, attempt=attempt@entry=0, retry=retry@entry=0x7ffc81c0e78f)
           at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5028
       #11 0x000000000097d06c in yb_exec_query_wrapper (exec_context=exec_context@entry=0x2582ffdc6000, restart_data=restart_data@entry=0x7ffc81c0e7d0,
           functor=functor@entry=0x97e028 <yb_exec_simple_query_impl>, functor_context=functor_context@entry=0x2582ffdc6128)
           at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5052
       #12 0x000000000097d0bf in yb_exec_simple_query (query_string=query_string@entry=0x2582ffdc6128 "select * from t;", exec_context=exec_context@entry=0x2582ffdc6000)
           at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5075
       #13 0x000000000097fe7f in PostgresMain (dbname=<optimized out>, username=<optimized out>) at ../../../../../../src/postgres/src/backend/tcop/postgres.c:5794
       #14 0x00000000008c8349 in BackendRun (port=0x2582ff8403c0) at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4791
       #15 BackendStartup (port=0x2582ff8403c0) at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:4491
       #16 ServerLoop () at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1878
       #17 0x00000000008caa4a in PostmasterMain (argc=argc@entry=25, argv=argv@entry=0x2582ffdc01a0) at ../../../../../../src/postgres/src/backend/postmaster/postmaster.c:1533
       #18 0x0000000000804b9d in PostgresServerProcessMain (argc=25, argv=0x2582ffdc01a0) at ../../../../../../src/postgres/src/backend/main/main.c:208
       #19 0x0000000000804bbd in main ()

       294             /*
       295              * close heap scan
       296              */
       297             if (tsdesc != NULL)
       298                     table_endscan(tsdesc);

   Reason is initial merge 55782d5
   incorrectly merges end of ExecEndYbSeqScan.  Upstream PG
   9ddef36278a9f676c07d0b4d9f33fa22e48ce3b5 removes code, but initial
   merge duplicates lines.  Remove those lines.

Test Plan:
Apply the following patch to activate YB Seq Scan:

    diff --git a/src/postgres/src/backend/optimizer/path/allpaths.c b/src/postgres/src/backend/optimizer/path/allpaths.c
    index 8a4c38a965..854d84a648 100644
    --- a/src/postgres/src/backend/optimizer/path/allpaths.c
    +++ b/src/postgres/src/backend/optimizer/path/allpaths.c
    @@ -576,7 +576,7 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
                     else
                     {
                         /* Plain relation */
    -                    if (IsYBRelationById(rte->relid))
    +                    if (false)
                         {
                             /*
                              * Using a foreign scan which will use the YB FDW by

On almalinux 8,

    ./yb_build.sh fastdebug --gcc11
    pg15_tests/run_all_tests.sh fastdebug --gcc11 --sj --sp --scb

fails the following tests:

- test_D29546
- test_pg15_regress: yb_pg15
- test_types_geo: yb_pg_box
- test_hash_in_queries: yb_hash_in_queries

Manually check to see that they are due to YB Seq Scan explain output
differences.

Reviewers: aagrawal, tfoucher

Reviewed By: tfoucher

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D31139
jasonyb pushed a commit that referenced this issue Jun 11, 2024
@ibrarahmad
Major refactoring to support multiple new features.
179e99d 
Issue - (#16): PG-112: Change the column name "ip" to "client_ip" for readability purpose.
Issue - (#17): PG-111: Show all the queries from complete and incomplete buckets.
Issue - (#18): PG-108: Log the bucket start time.
Issue - (#19): PG-99: Response time histogram.
Issue - (#20): PG-97: Log CPU time for a query.
Issue - (#21): PG-96: Show objects(tables) involved in the query.
Issue - (#22): PG-93: Retain the bucket, and don't delete the bucket automatically.
Issue - (#23): PG-91: Log queries of all the databases.
Issue - (#24): PG-116: Restrict the query size.
Issue - (#3) : README file update.
devansh-ism pushed a commit to devansh-ism/yugabyte-db that referenced this issue Jul 19, 2024
@rcoup
Improve JSON encoding:
003132b 
* use the postgres builtin escape_json() function
* escape all type, schema, table, field names. Add tests. [yugabyte#35]
* leave `\x` alone unless it's a bytea prefix [yugabyte#23]
* escape generic logical decoding messages with tests
devansh-ism pushed a commit to devansh-ism/yugabyte-db that referenced this issue Jul 19, 2024
Initial commit
806f637 
Summary:
Copyright update.

Initial commit.

The plugin was first named json_decoding_plugin but changed the name
after discussion on -hackers. It is a POC to illustrate that it is
possible for changeset extraction work just fine. The changeset
extraction feature is already in development, hence, it is expected that
this code changes a lot until the feature is set in stone.

It is based on:

repository: git:https://git.postgresql.org/git/users/andresfreund/postgres.git
branch: xlog-decoding-rebasing-remapping
commit: 6f819a68cd10938df126ee44a3cfef7f457c8e1b

Generate a long bytea instead of explicitly writing a long one.

It reduces the sql file size. Suggested by Merlin Moncure.

In commit ea177a3ba7a7901f6467eadb0a407e03d46462fd, the internal structure was changed.

Unbreak regression tests.

Remove contrib dependency.

The old Makefile relies heavily on been in the contrib tree. While
analysing the ethansf pull request, I realized that that premise does
not hold water because you could build it without a source code
tree.

Although I didn't have used his patch [1], this code was inspired on it.

[1] eulerto/wal2json#1

Fix regression tests.

The regression tests was very unstable. The main reason is the
associated LSN in some CONTEXT messages. Adding VERBOSITY setting to
supress those CONTEXT messages.

Update copyright.

Include LSN position for the next transaction.

This feature was discussed with @ethansf and is useful for tool that
consumes WAL using pg_recvlogical and similar tools. The LSN is
important to mark the apply progress. It stores the LSN pointing to the
end of commit record + 1. Hence, if the consuming tool crashes while
applying a transaction (JSON document), it can restart the
pg_recvlogical streaming from the last saved LSN position. This is
essential for such consuming tool to work properly.

Although, I did not have used Ethans' patch, this piece of code is
based on his idea.

Rename "next lsn" JSON key.

It is a comestic change because you can access values like:

foo['next lsn']

The other form (foo.bar) is not possible because of the blank space.

Fix handling of nonexistant columns in old tuple versions.

This fix was discoverd in bug yugabyte#13470 and commit
d47a1136e441cebe7ae7fe72d70eb8ce278d5cd6 fixes it.

pretty print option.

The new parameter 'pretty-print' chooses between a compact (without
spaces) or a pretty-print format.

The default changed to compact format since tools usually transform the
JSON objects.

Control how the stream is written.

The new parameter 'write-in-chunks' controls how to write in the
stream. If true, write occurs per tuple; else it writes at the end
of transaction.

The default is true. Hence, the behavior is the same as prior code.

Add a real README.

README has instructions to build and install wal2json. Also, there are
examples of use.

Add Coverity Scan Badge

Replace palloc with palloc0

Avoid garbage on JsonDecodingData.

Fixed tests results

Give an error instead of a warning in case of bad param

Added gitignore

Ignore .o files

Error message on unknown params reworded

Merge pull request yugabyte#13 from dvarrazzo/gitignore

Added gitignore

Merge pull request yugabyte#12 from dvarrazzo/err-on-bad-option

Give an error instead of a warning in case of bad param

Merge pull request yugabyte#11 from dvarrazzo/fix-tests

Fixed tests results

Default for write-in-chunks changed to false

Default for include-xids changed to false

style fix

Update copyright year information

Dropped include-xids=0 from test suite

Avoid leaking memory when we have some can-not-happen situations

Fix LICENSE

Forgot to replace organization.

Merge pull request yugabyte#18 from dvarrazzo/better-defaults

Better defaults

Use '1' and '0' instead of 't' and 'f' on tests

Forgot to add some static on functions

Daniele Varrazzo

Support generic WAL messages for logical decoding

This feature allows software to insert data into WAL stream that can be
read by wal2json. Those messages could be useful to control replication,
for example. Messages can be sent as transactional or not. Non-transactional messages mean that it is sent even if the transaction is rollbacked.

There was a PR yugabyte#20 for this same feature but I didn't use it. Indeed,
this code was dusty in my computer for a few months.

NOTE: 'message' test will fail on <= 9.5 because this feature was coded
in 9.6 (I don't want to complicate Makefile).

Fix generic WAL messages

As spotted in issue yugabyte#28, I committed a code that was not supposed to be
part of the original commit 645ab69.

Thanks to Julien Rouhaud (@rjuju) to bring it up.

Support type name with modifier

Issue yugabyte#31 asked for type name with type modifier (if available). This
commit implemented type modifier inclusion by default. It is possible to
use the old type name, if you use the option 'include-typmod' as 'false'.

I'll change the regression tests to use the new format later. I've added
a new test (typmod) to exercise this new option.

Fix a description

Remove superfluous includes

rel.h includes pg_class.h, index.h, and relcache.h. logical.h includes
output_plugin.h. All of the others are not necessary.

TupleDescAttr() support

Commit d34a74dd064af959acd9040446925d9d53dff15b introduced
TupleDescAttr(tupdesc, i) in back branches (9.4.14, 9.5.9, 9.6.5, 10). If the
version supports it, use this macro. This change will unbreak (future) version
11.

Support type oids

Issue yugabyte#37 asked for type oids in the output because psycopg2 uses it
directly for internal type manipulation. Although @lionel-panhaleux had
provided PR yugabyte#38, I didn't like the way it was implemented mainly because
it mixes options. The new option 'include-type-oids' has default to
false. I didn't provide tests for this feature.

Support not-null constraints information

Issue yugabyte#44 asked for not-null constraints information because the other
side could benefit from this. The new option 'include-not-null' will add
another name/value pair (columnoptionals) that contains false for
not-null constraint (it means that the value is not optional) or true,
otherwise. The default is false.

Improve JSON encoding: * use the postgres builtin escape_json() function * escape all type, schema, table, field names. Add tests. [yugabyte#35] * leave `\x` alone unless it's a bytea prefix [yugabyte#23] * escape generic logical decoding messages with tests

Update call to AllocSetContextCreate for PostgreSQL 9.6+

Merge pull request yugabyte#55 from davidfetter/pg_96_plus

Update call to AllocSetContextCreate for PostgreSQL 9.6+

Filter out tables

The new parameter 'filter-tables' is a comma separated value that
contains schema.table elements. Both schema and table can be * that
denotes all. Hence, *.table_1 means table_1 in all schemas and
schema_1.* means all tables in schema_1.

Some characters (space, single quote, comma, period, asterisk) have
special treatment because they are part of the comma separated value. If
any of these characters is part of the schema or table name, add a
backslash (\) before it. For example, table public."foo bar" should be
specified as 'public.foo\ bar'.

Select tables to retrieve data

The new parameter 'add-tables' select only data from those tables. Like
parameter 'filter-tables', it is a comma separated value that contains
schema.table elements. Both schema and table can be * that denotes all.
Escape also works for special characters. By default, all tables in all
schemas are selected.

Merge pull request yugabyte#40 from koordinates/encoding

Improve JSON encoding

Preserve unchanged toast values by default. Use `include-unchanged-toast=0` option to suppress.

Merge pull request yugabyte#50 from koordinates/include-unchanged-toast

Preserve unchanged toast values by default.

This commit introduces a behavior change since TOAST values will always be output from now on. If you want the previous behavior, add include-unchanged-toast=0.

Style fixes for last commit

Copyright update

Document parameters

Update examples

The examples don't reflect the defaults options. Also, rename tables in
SQL functions example (avoid table conflict with the other test).

Build wal2json out of the tree on Windows

Build out of the tree is a common task in Unix-like systems (PostgreSQL
has PGXS but it doesn't support Windows). This commit adds a project
file to build out of the tree on Windows. I also add minimal
instructions to build on Windows. I tested wal2json on MS Visual Studio
2017 (version 15.6.4) on a x64 platform.

Improve style

Fix oversight in copyright message

LICENSE is correct but C file has wrong copyright message.

Tweak message test

Per PR yugabyte#59, replace bytea value with valid data.

Sanity checks after selective table options

The sanity checks should be run after filter-tables and add-tables
options to avoid a bunch of unsolicited warnings (per issue yugabyte#64).

Refactor wal2json output

Almost all writes were handled in two different ways: with or without
pretty print. It means more lines of code and an error-prone code. The
new code avoids all of the pretty-print tests and also uses only one
setence per output. It reduces the code by more than 150 lines.

I also fix some comments (use only C89 comments) and improve style.

Revert include-unchanged-toast option

Per discussion in issue yugabyte#74, we can't rely on access unchanged TOAST
data because it is not in the WAL stream.

It reverts commits 947043e
ce82d73 and
d86a13b .

Add an error message to include-unchanged-toast parameter

Per issue yugabyte#74, parameter include-unchanged-toast wasn't safe, hence,
remove it. Since it is part of release 1.0, deprecate this option for
the next release.

Make wal2json logging less chatty

Suppress the "XXX argument is null" message. Decrease the TOAST message
level from WARNING to DEBUG1. Also, decrease some debug message levels.

Improve perf on deletes when repl ident full

Merge pull request yugabyte#82 from rkrage/repl-ident-full-perf

Improve perf on deletes when repl ident full

Add tests for include-timestamp option

Add tests for include-lsn option

Add tests for include-xids option

Merge pull request yugabyte#94 from paulczajka/add-tests

Add tests for timestamp, lsn, xids options

Avoid null pointer dereference with empty filter-tables

As spotted by clang:
wal2json.c:321:21: warning: Dereference of null pointer
                        rawstr = pstrdup(strVal(elem->arg));
                                         ^~~~~~~~~~~~~~~~~
/usr/include/postgresql/11/server/nodes/value.h:54:20: note: expanded from macro 'strVal'

Add option format-version

This option defines which format to use. Only version 1 is currently
supported. New formats will be available.

Note include-unchanged-toast is deprecated

Merge pull request yugabyte#102 from benjie/patch-1

Note `include-unchanged-toast` is deprecated

Merge pull request yugabyte#97 from xrmx/nullfiltertables

Avoid null pointer dereference with empty filter-tables

remove empty line

Add 'cd' command to README

Merge pull request yugabyte#107 from benjie/patch-2

Add 'cd' command to README

PATH should contain the parent folder of pg_config

Merge pull request yugabyte#108 from benjie/patch-3

PATH should contain the parent folder of pg_config

Fix add-tables to filter single-character schemas

Add extra_float_digits to tests

Postgres commit 02ddd499322ab6f2f0d58692955dc9633c2150fc for v12 and
later, changes the default number of trailing digits output for double
precision. Add extra_float_digits to keep regression tests stable.

Filter messages by prefix

The new parameter 'filter-msg-prefixes' filters messages with those
prefixes. It a comma separated value. By default no messages are
filtered. Per off-list discussion with @martinmarques

Updated copyright to 2019

Add messages by prefix

The new parameter 'add-msg-prefixes' adds only messages with these
prefixes. It is a comma separated value. By default all messages are
included. wal2json enforces this rule after 'filter-msg-prefixes'. Per
off-list discussion with @martinmarques

Merge pull request yugabyte#123 from davidfetter/copyright_2019

Updated copyright to 2019

Update instructions

Since version 10, we don't need to adjust parameters. Also, pg_hba.conf
changes its behavior. It matches normal connections instead of
(physical) replication connections.

Merge pull request yugabyte#113 from benjie/patch-4

Fix add-tables to filter single-character schemas

New wal2json format

This is a new format for wal2json. You can choose it using
option 'format-version' = 2. This format is completely different from
version 1.

Features are:

* one JSON per tuple;
* each JSON has an "action" (BEGIN, COMMIT, INSERT, UPDATE, DELETE,
MESSAGE);
* one (optional) JSON object for BEGIN/COMMIT;
* BEGIN contains xid, timestamp, and lsn;
* COMMIT contains xid, timestamp, and lsn;
* INSERT/UPDATE/DELETE contains lsn, schema, table, columns, identity;
* "columns" and "identity" are arrays of elements;
* each "columns" element is an object that contains name, type, value,
and optional;
* "identity" is an array of elements (REPLICA IDENTITY for UPDATE /
DELETE statements) that contains name, type, and value;
* MESSAGE contains xid, timestamp, lsn, transactional, prefix, and
content.

This new format solves the big transaction issue (that consumes a lot of
memory) since tuples aren't accumulate until the end of transaction to
be written. Users can control transactions using option
include-transaction that will emit a JSON at the beginning of the
transaction and another one at the end of the transaction.

Fix tests

Postgres 12 breaks random() predictability. Since we use the same tests
for all versions, replace random() with a sequence.

Improve regression test

message regression test used to fail with 9.4 and 9.5. That's because
message API is available in 9.6 and later. Filter out this test if the
major version is 9.4 or 9.5.

Add parameter include-origin

This parameter includes replication origin. Default is false. Format
version 2 adds origin information in BEGIN, COMMIT, CHANGE, and MESSAGE.
In format version 1, origin is also available. Since replication origin
was introduced in 9.5, 9.4 does not print origin information.

Support TRUNCATE command for logical decoding

This feature includes TRUNCATE commands into wal2json output. It was
introduced in PostgreSQL 11 which means that wal2json will support it
only for this version and on. Format version 2 will output it by
default. Format version 1 won't (maintain backward compatibility). (A
new option will be added to select actions -- insert, update, delete,
truncate -- and it should be possible to output TRUNCATE commands in
version 1 too).

Option actions

This new feature let users select actions they want to receive. Actions
supported are insert, update, delete, and truncate. This actions are
similar to PUBLICATION command.

Format version 1 won't enable truncate by default (to maintain backward
compatibility). Format version 2 will enable all options by default.

A new test was added (run only on Postgres v11 or later).  That's
because test includes truncate and it will only supported by v11 or
later.

Adjust default format version

Format version 1 will still remain the default version. wal2json does
not require a format version until a few commits ago (with no new
version released) and use a new format version will break calls that
does not inform format-version parameter. Let's give some time to users
adopt format-version parameter.

format-version 2 example

This new example shows the output for format-version 2.

Fix license oversight

The wal2json license is BSD 3-clause, however, there is a sentence in the
README that says it is released under PostgreSQL license (it is not). This is
an oversight in the commit 0b653ee.

Update copyright

Adjust instructions

Use default max_replication_slots value as suggestion for examples /
tests.

Fix oversight in option actions

This oversight was introduced in commit
aa431a9.

Issue yugabyte#148 reported by @dko-slapdash.

Fix memory leak while using add-tables and filter-tables

This issue was reported in PR yugabyte#147 by @haizz. The proposed fix was
incomplete (as I pointed out) and I fixed add-tables option and also
format 2 (add-tables and filter-tables).

Fix segfault on 32 bits

I forgot to pass a Datum and I'm surprised it haven't crashed yet.  This
commit fixes issue yugabyte#142.

Reported by @df7cb and confirmed by @cpaelzer.

Use correct format specifier

uint64 should use UINT64_FORMAT instead of %lu because format specifier
depends on a type probe. It suppresses a compiler warning.

Move some debug code for general function

Some debug messages were only printed if we are using format 1, let's
move them to general function.

Add macro for backward compatibility

UInt64GetDatum was introduced in 9.6. I forgot to test old stable
versions and didn't get it. Fortunately, @df7cb reported it.

Add parameter include-domain-data-type

This parameter replaces domain name with its underlying data type.
Default is false (backward compatibility). Both formats (1 and 2) are
supported. It is useful if you want to expose the "real" data type.

struct ReorderBufferTXN changed

Some struct members (including has_catalog_changes) was turned into a
single variable since v13 (cf postgres commit
a7b6ab5db1d35438112f74f3531354ddd61970b5).

Rephrase include-unchanged-toast docs

This parameter was deprecated some time ago. However, its description is
confusing users. Let's add a blink alert to NOT use it. It will
eventually be removed.

Add expected file for parameter include-domain-data-type

Commit e33df94 forgot to add expected
file.

Add parameter filter-origins

This parameter is useful in multidirectional replication solutions. It
can prevent replicating some data back and forth. Although, each
transaction has origin information, filter using this callback is more
efficient. This callback is available in 9.5+.

Improve Linux installation instructions

Since there are some issues that are opened from time to time, I add a
few instructions to Linux distros. I add instructions to install the
available packages from Debian/Ubuntu and RHEL/CentOS. I also write some
instructions to compile on those OSes. I hope it should clarify doubts
about installation.

Fix an oversight for old minor versions

Format 2 (1b0cbac) forgot to consider
the workaround I applied to TupleDescAttr
(5352cc4).  Since I did not test with
old minor versions, this bug was not caught by tests. This was reported
in issue yugabyte#162.

Improve configuration section

Add a comment explaining which parameters need adjust based on the
version users are using. Version 10 or later just need to change
wal_level, for example. Though, old versions have to adjust a few
parameters to use logical replication.

Add parameter include-column-positions

This parameter is useful to detect schema changes (since attnum are
numbered from 1 up). Both formats (1 and 2) are supported although
element names are different ("columnpositions" for format 1 and
"position" for format 2). Default is false.

Issue yugabyte#160

Add parameter include-default

This parameter adds "columndefaults" that contains all defaults values
(same as pg_get_expr). Both formats (1 and 2) are supported although
element names are different ("columndefaults" for format 1 and "default"
for format 2). Each default value is represented as string because
it can be an expression. Tests are included. Default is false.

I also added prototypes to 2 static functions.

Add parameter include-pk

This parameter adds primary key information if it is available. Both
formats (1 and 2) are supported. Each "pk" object provides column name
and its data type. Tests are included. Default is false.

Fix an oversight in format 2: nextlsn

Format 2 (1b0cbac) forgot to add
nextlsn. Format 1 includes this information if include-lsn is provided.
This information can be useful for applications that use LSN position as
a restart point.

Fix package name

As pointed by @MaxmaxmaximusAWS Debian/Ubuntu package name was wrong.
It fixes yugabyte#175.

Update instructions to Postgres 13

Since Postgres 13 was released a few days ago, let's update wal2json
instructions. I also replaced wal2json version with the latest version
(2.3).

Improve pretty print

Replace strcpy() and strncpy() with an assignment.

Improve documentation

Add 'message' usage to the examples. In comparison to the widely used
output plugins such as pgoutput and pglogical 2, wal2json is the only
plugin that supports message_cb.

Fix table names in the example

I renamed the tables in the first example but forgot to change the
script.

Remove unused code

Don't rely on index attribute names for primary key

If you rename a column that is part of the primary key, the attname for
the primary key index isn't renamed. Someone could argue that it is a
Postgres bug (I think it is a catalog inconsistency that could probably
be fixed eventually) but it seems the cure is worse than the disease
(see [1]).

The code was refactored to avoid comparison by attribute name, instead
it uses RelationGetIndexAttrBitmap() that obtains a bitmap of index
attribute numbers based on its kind (primary key, identity, ...).

Closes yugabyte#11

[1] https://www.postgresql.org/message-id/CAA8M49r%3D1XoE27tQ08sAPhft_ayBu4Vvib%2BubwX4SRqtUGJ%2B3g%40mail.gmail.com

Combine common code into functions

Instead of repeating code into different wal2json format output
functions, combine the common code into functions and use them.

Add docs on include-origin option.

document include-transaction option

Filter is not applied for TRUNCATE

Filter is failing to exclude tables that matchs the filter. Filter code
was refactored at commit a96dd31 but I
forgot to replace the filter in the truncate function. I include a
regression test to cover this code.

Merge pull request yugabyte#207 from olirice/master

Document include-transaction option

Avoid duplicate double quotes for type names

When an internal function already returns a type name with double
quotes, doesn't add another one.

Fix yugabyte#200

Stamp 2.4

default value was not always printed in v1

This bug prevents a default value to be printed in v1 after a TRUNCATE
statement.

Reported by @ls-guillaume-rebesche

Fixes yugabyte#225

Parameter write-in-chunks is only used for format version 1

If you set format-version to 2, parameter write-in-chunks has no effect.
Document this behavior.

Fixes yugabyte#228

Add Postgres 14 to the installation instructions

Refer to the latest Postgres version in the instructions.

Update copyright year

Add include-type-oids support in format-version 2

unbreak test on old versions

'default' test uses TRUNCATE that is not supported in v10 and previous
versions. Add an alternate test file to cover this case.

Array type does not print the correct type name

If the base type requires quotes and the type is an array, wal2json is
not quoting the whole type name. Instead, it was not considering the
brackets as part of the type name. Fix both format versions.

Fixes yugabyte#233

Lag tracking support

See lag times in pg_stat_replication.

Add PostgreSQL 15 support

Merge pull request yugabyte#247 from Naros/postgres15-compatibility

Add PostgreSQL 15 support

Stamp 2.5

Add option to output numeric data types as string. (yugabyte#255)

Add option to output numeric data types as string.

Data types like `numeric`, `real`, `double precision` supports `Infinity`,
`-Infinity` and `NaN` values. Currently these values output as `null` because
JSON specification does not recognize them as valid numeric values. This will
create problems for the users of wal2json who need these values to maintain
data integerity.

invalid JSON for non-transaction message in v1 (yugabyte#266)

Non-transactional messages contain only one object. It means comma
should not be provided for such JSON objects. If you are sending a
non-transactional message into a transaction, it should guarantee that
comma is only emitted for transactional commands. Hence, the previous
check is weak and it should also check for transactional information.

Fixes yugabyte#266

Correct DELETED tuple reference pg_decode_change_v1 (yugabyte#252)

Avoid variable shadowing

Fix typo

Fix lag tracking support for large transactions

Postgres fixed some time ago a logical replication timeout during large
transactions (commit
postgres/postgres@f95d53e). For
Postgres 16, a proper fix was added in the logical decoding but the
previous versions (10 to 14) a new function
(update-replication_progress) was added. It is the same function used by
pgoutput plugin.

Fixes yugabyte#270

Add version

Packages might rely on the version from source code instead of
extracting the version from the tarball file name.

If the WAL2JSON_VERSION contains "dev" suffix it means it is not a
released version. WAL2JSON_VERSION_NUM changes only for release. After a
release, the next commit will add the "dev" suffix that will be removed
only for the next release.

Replace tab with space

Update .gitignore

Fix include-pk when replica identity is full

In format 1, the primary key information was not included even though
the include-pk option is enabled. It happens when the table has a
primary key and the replica identity is full. Format 2 was not affected.

Fixes yugabyte#273

Update instructions with the latest PostgreSQL version

wal2json supports up to the latest supported version: 16.

Fix Red Hat package name

Update copyright year

Add PostgreSQL 17 support

Postgres 17 changed the ReorderBufferChange data structure (commit
postgres/postgres@08e6344).

Update build infrastructure for Windows

It updates MS Visual Studio from 2017 (v141) to 2022 (v143). It also
changes the target OS from 8.1 to 10.0 because Windows 8 was EOL for
some time (Windows 10 is also deprecated 3 months ago but since it is
the one I used to test these builds, I kept it. Feel free to update
WindowsTargetPlatformVersion if you are using a newer Windows version.)

It also includes a solution file (wal2json.sln -- although there aren't
multiple project files) that you can also use to trigger the build using
the following command:

msbuild wal2json.sln /verbosity:normal /p:Configuration=Release

I tested these builds with Postgres 10 and later on x86_64.

Stamp 2.6

Add 'src/postgres/third-party-extensions/wal2json/' from commit '75629c2e1e81a12350cc9d63782fc53252185d8d'

git-subtree-dir: src/postgres/third-party-extensions/wal2json
git-subtree-mainline: 66ed3a5
git-subtree-split: 75629c2

Test Plan: jenkins: compile only

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D36690
myang2021 added a commit that referenced this issue Aug 8, 2024
@myang2021
[#13358] YSQL: Fix DDL atomicity stress test failure in tsan build
bd4874b 
Summary:
The DDL atomicity stress tests failed more on pg15 branch with an error like:

```
WARNING: ThreadSanitizer: data race (pid=180911)
  Write of size 8 at 0x7b2c000257b8 by thread T17 (mutexes: write M0):
    #0 profile_open_file prof_file.c (libkrb5.so.3+0xf45b3)
    #1 profile_init_flags <null> (libkrb5.so.3+0xfb056)
    #2 k5_os_init_context <null> (libkrb5.so.3+0xe5546)
    #3 krb5_init_context_profile <null> (libkrb5.so.3+0xabc90)
    #4 krb5_init_context <null> (libkrb5.so.3+0xabbd5)
    #5 krb5_gss_init_context init_sec_context.c (libgssapi_krb5.so.2+0x448da)
    #6 acquire_cred_from acquire_cred.c (libgssapi_krb5.so.2+0x39159)
    #7 krb5_gss_acquire_cred_from acquire_cred.c (libgssapi_krb5.so.2+0x39072)
    #8 gss_add_cred_from <null> (libgssapi_krb5.so.2+0x1fcd3)
    #9 gss_acquire_cred_from <null> (libgssapi_krb5.so.2+0x1f69d)
    #10 gss_acquire_cred <null> (libgssapi_krb5.so.2+0x1f431)
    #11 pg_GSS_have_cred_cache ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-gssapi-common.c:68:10 (libpq.so.5+0x543fe)
    #12 PQconnectPoll ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-connect.c:2909:22 (libpq.so.5+0x359ca)
    #13 connectDBComplete ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-connect.c:2241:10 (libpq.so.5+0x30807)
    #14 PQconnectdb ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-connect.c:719:10 (libpq.so.5+0x30af1)
    #15 yb::pgwrapper::PGConn::Connect(string const&, std::chrono::time_point<yb::CoarseMonoClock, std::chrono::duration<long long, std::ratio<1l, 1000000000l>>>, bool, string const&) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_utils.cc:348:24 (libpq_utils.so+0x13c5b)
    #16 yb::pgwrapper::PGConn::Connect(string const&, bool, string const&) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_utils.h:254:12 (libpq_utils.so+0x1a77e)
    #17 yb::pgwrapper::PGConnBuilder::Connect(bool) const ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_utils.cc:743:10 (libpq_utils.so+0x1a77e)
    #18 yb::pgwrapper::LibPqTestBase::ConnectToDBAsUser(string const&, string const&, bool) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_test_base.cc:54:6 (libpg_wrapper_test_base.so+0x26f34)
    #19 yb::pgwrapper::LibPqTestBase::ConnectToDB(string const&, bool) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_test_base.cc:44:10 (libpg_wrapper_test_base.so+0x26b1e)
    #20 yb::pgwrapper::LibPqTestBase::Connect(bool) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_test_base.cc:40:10 (libpg_wrapper_test_base.so+0x26b1e)
    #21 yb::pgwrapper::PgDdlAtomicityStressTest::Connect() ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc:147:25 (pg_ddl_atomicity_stress-test+0x136d6c)
    #22 yb::pgwrapper::PgDdlAtomicityStressTest::TestDdl(std::vector<string, std::allocator<string>> const&, int) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc:165:15 (pg_ddl_atomicity_stress-test+0x136df5)
    #23 yb::pgwrapper::PgDdlAtomicityStressTest_StressTest_Test::TestBody()::$_2::operator()() const ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc:316:5 (pg_ddl_atomicity_stress-test+0x13d2eb)
```

It appears that the function `yb::pgwrapper::LibPqTestBase::Connect` isn't
thread safe. I restructured the code to make the connections in a single thread
and then pass them to various concurrent threads for testing.
Jira: DB-2996

Test Plan:
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/0 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/1 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/2 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/3 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/4 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/5 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/6 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/7 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/8 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/9 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/10 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/11 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/12 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/13 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/14 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/15 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/16 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/17 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/18 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/19 --clang17

Verified that no more tsan errors.

Reviewers: fizaa

Reviewed By: fizaa

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D37111
myang2021 added a commit that referenced this issue Aug 8, 2024
@myang2021
[BACKPORT 2024.1][#13358] YSQL: Fix DDL atomicity stress test failure…
3cb5580 
… in tsan build

Summary:
The DDL atomicity stress tests failed more on pg15 branch with an error like:

```
WARNING: ThreadSanitizer: data race (pid=180911)
  Write of size 8 at 0x7b2c000257b8 by thread T17 (mutexes: write M0):
    #0 profile_open_file prof_file.c (libkrb5.so.3+0xf45b3)
    #1 profile_init_flags <null> (libkrb5.so.3+0xfb056)
    #2 k5_os_init_context <null> (libkrb5.so.3+0xe5546)
    #3 krb5_init_context_profile <null> (libkrb5.so.3+0xabc90)
    #4 krb5_init_context <null> (libkrb5.so.3+0xabbd5)
    #5 krb5_gss_init_context init_sec_context.c (libgssapi_krb5.so.2+0x448da)
    #6 acquire_cred_from acquire_cred.c (libgssapi_krb5.so.2+0x39159)
    #7 krb5_gss_acquire_cred_from acquire_cred.c (libgssapi_krb5.so.2+0x39072)
    #8 gss_add_cred_from <null> (libgssapi_krb5.so.2+0x1fcd3)
    #9 gss_acquire_cred_from <null> (libgssapi_krb5.so.2+0x1f69d)
    #10 gss_acquire_cred <null> (libgssapi_krb5.so.2+0x1f431)
    #11 pg_GSS_have_cred_cache ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-gssapi-common.c:68:10 (libpq.so.5+0x543fe)
    #12 PQconnectPoll ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-connect.c:2909:22 (libpq.so.5+0x359ca)
    #13 connectDBComplete ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-connect.c:2241:10 (libpq.so.5+0x30807)
    #14 PQconnectdb ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/../../../../../../src/postgres/src/interfaces/libpq/fe-connect.c:719:10 (libpq.so.5+0x30af1)
    #15 yb::pgwrapper::PGConn::Connect(string const&, std::chrono::time_point<yb::CoarseMonoClock, std::chrono::duration<long long, std::ratio<1l, 1000000000l>>>, bool, string const&) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_utils.cc:348:24 (libpq_utils.so+0x13c5b)
    #16 yb::pgwrapper::PGConn::Connect(string const&, bool, string const&) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_utils.h:254:12 (libpq_utils.so+0x1a77e)
    #17 yb::pgwrapper::PGConnBuilder::Connect(bool) const ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_utils.cc:743:10 (libpq_utils.so+0x1a77e)
    #18 yb::pgwrapper::LibPqTestBase::ConnectToDBAsUser(string const&, string const&, bool) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_test_base.cc:54:6 (libpg_wrapper_test_base.so+0x26f34)
    #19 yb::pgwrapper::LibPqTestBase::ConnectToDB(string const&, bool) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_test_base.cc:44:10 (libpg_wrapper_test_base.so+0x26b1e)
    #20 yb::pgwrapper::LibPqTestBase::Connect(bool) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/libpq_test_base.cc:40:10 (libpg_wrapper_test_base.so+0x26b1e)
    #21 yb::pgwrapper::PgDdlAtomicityStressTest::Connect() ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc:147:25 (pg_ddl_atomicity_stress-test+0x136d6c)
    #22 yb::pgwrapper::PgDdlAtomicityStressTest::TestDdl(std::vector<string, std::allocator<string>> const&, int) ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc:165:15 (pg_ddl_atomicity_stress-test+0x136df5)
    #23 yb::pgwrapper::PgDdlAtomicityStressTest_StressTest_Test::TestBody()::$_2::operator()() const ${YB_SRC_ROOT}/src/yb/yql/pgwrapper/pg_ddl_atomicity_stress-test.cc:316:5 (pg_ddl_atomicity_stress-test+0x13d2eb)
```

It appears that the function `yb::pgwrapper::LibPqTestBase::Connect` isn't
thread safe. I restructured the code to make the connections in a single thread
and then pass them to various concurrent threads for testing.
Jira: DB-2996

Original commit: bd4874b / D37111

Test Plan:
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/0 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/1 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/2 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/3 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/4 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/5 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/6 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/7 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/8 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/9 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/10 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/11 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/12 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/13 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/14 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/15 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/16 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/17 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/18 --clang17
./yb_build.sh tsan --cxx-test pgwrapper_pg_ddl_atomicity_stress-test --gtest_filter PgDdlAtomicityStressTest/PgDdlAtomicityStressTest.StressTest/19 --clang17

Verified that no more tsan errors.

Reviewers: fizaa

Reviewed By: fizaa

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D37167
amitanandaiyer added a commit that referenced this issue Sep 5, 2024
@amitanandaiyer
[#23747] MetaCache: Callback should not be called while holding the lock
c770d79 
Summary:
Call callback in ScopeExit block only. Not while holding the lock.

Without this fix, it is possible that a thread can get into a deadlock, trying to request a shared_lock on a mutex, while already holding an exclusive lock on the same mutex:

This deadlock can be triggered if there are active read/write requests to a Table (from more than 1 thread)  right after the table had a tablet-split.

 If there is only 1 thread, it is unlikely to run into the deadlock, as the thread notices -- as part of the callback -- that the table's partition info is stale. Having a different thread refresh the partition version before the main thread checks if the table version is stale, is likely necessary to trigger the stack trace seen below.

e.g:
```
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x00005640c3eb441b in std::__1::shared_timed_mutex::lock_shared() ()
#2  0x00005640c3ffcbff in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>, yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
#3  0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#4  0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#5  0x00005640c401aa76 in yb::client::(anonymous namespace)::BatcherFlushDone(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig) ()
#6  0x00005640c401b371 in boost::detail::function::void_function_obj_invoker1<std::__1::__bind<void (*)(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig), std::__1::shared_ptr<yb::client::internal::Batcher> const&, std::__1::placeholders::__ph<1> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig&>, void, yb::Status const&>::invoke(boost::detail::function::function_buffer&, yb::Status const&) ()
#7  0x00005640c3f70398 in yb::client::internal::Batcher::Run() ()
#8  0x00005640c3f72656 in yb::client::internal::Batcher::FlushFinished() ()
#9  0x00005640c3f74a4d in yb::client::internal::Batcher::TabletLookupFinished(yb::client::internal::InFlightOp*, yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> >) ()
#10 0x00005640c3f759bc in std::__1::__function::__func<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0, std::__1::allocator<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0>, void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>::operator()(yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&) ()

#11 0x00005640c3fff05d in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>,  yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
** Is holding an exclusive lock in MetaCache::LookupTabletByKey/DoLookupTabletByKey **

#12 0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#13 0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#14 0x00005640c4017130 in yb::client::YBSession::FlushAsync(boost::function<void (yb::client::FlushStatus*)>) ()
#15 0x00005640c5225a0c in yb::tserver::PgClientServiceImpl::Perform(yb::tserver::PgPerformRequestPB const*, yb::tserver::PgPerformResponsePB*, yb::rpc::RpcContext) ()
#16 0x00005640c51c4487 in std::__1::__function::__func<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20, std::__1::allocator<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20>, void (std::__1::shared_ptr<yb::rpc::InboundCall>)>::operator()(std::__1::shared_ptr<yb::rpc::InboundCall>&&) ()
#17 0x00005640c51d374f in yb::tserver::PgClientServiceIf::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#18 0x00005640c4f5f420 in yb::rpc::ServicePoolImpl::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#19 0x00005640c4e845af in yb::rpc::InboundCall::InboundCallTask::Run() ()
#20 0x00005640c4f6e243 in yb::rpc::(anonymous namespace)::Worker::Execute() ()
#21 0x00005640c570ecb4 in yb::Thread::SuperviseThread(void*) ()
#22 0x00007f808b7c6694 in start_thread (arg=0x7f76d8caf700) at pthread_create.c:333
#23 0x00007f808bac341d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
```
Jira: DB-12651

Test Plan:
Jenkins
yb_build.sh --cxx-test ql-stress-test QLStressTest.ReproMetaCacheDeadlock

Reviewers: rthallam, hsunder, qhu, timur

Reviewed By: hsunder

Subscribers: svc_phabricator, ybase

Differential Revision: https://phorge.dev.yugabyte.com/D37706
amitanandaiyer added a commit that referenced this issue Sep 6, 2024
@amitanandaiyer
[BACKPORT 2024.1][#23747] MetaCache: Callback should not be called wh…
ec79d7b 
…ile holding the lock

Summary:
Original commit: c770d79 / D37706
Call callback in ScopeExit block only. Not while holding the lock.

Without this fix, it is possible that a thread can get into a deadlock, trying to request a shared_lock on a mutex, while already holding an exclusive lock on the same mutex:

This deadlock can be triggered if there are active read/write requests to a Table (from more than 1 thread)  right after the table had a tablet-split.

 If there is only 1 thread, it is unlikely to run into the deadlock, as the thread notices -- as part of the callback -- that the table's partition info is stale. Having a different thread refresh the partition version before the main thread checks if the table version is stale, is likely necessary to trigger the stack trace seen below.

e.g:
```
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x00005640c3eb441b in std::__1::shared_timed_mutex::lock_shared() ()
#2  0x00005640c3ffcbff in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>, yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
#3  0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#4  0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#5  0x00005640c401aa76 in yb::client::(anonymous namespace)::BatcherFlushDone(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig) ()
#6  0x00005640c401b371 in boost::detail::function::void_function_obj_invoker1<std::__1::__bind<void (*)(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig), std::__1::shared_ptr<yb::client::internal::Batcher> const&, std::__1::placeholders::__ph<1> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig&>, void, yb::Status const&>::invoke(boost::detail::function::function_buffer&, yb::Status const&) ()
#7  0x00005640c3f70398 in yb::client::internal::Batcher::Run() ()
#8  0x00005640c3f72656 in yb::client::internal::Batcher::FlushFinished() ()
#9  0x00005640c3f74a4d in yb::client::internal::Batcher::TabletLookupFinished(yb::client::internal::InFlightOp*, yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> >) ()
#10 0x00005640c3f759bc in std::__1::__function::__func<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0, std::__1::allocator<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0>, void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>::operator()(yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&) ()

#11 0x00005640c3fff05d in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>,  yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
** Is holding an exclusive lock in MetaCache::LookupTabletByKey/DoLookupTabletByKey **

#12 0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#13 0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#14 0x00005640c4017130 in yb::client::YBSession::FlushAsync(boost::function<void (yb::client::FlushStatus*)>) ()
#15 0x00005640c5225a0c in yb::tserver::PgClientServiceImpl::Perform(yb::tserver::PgPerformRequestPB const*, yb::tserver::PgPerformResponsePB*, yb::rpc::RpcContext) ()
#16 0x00005640c51c4487 in std::__1::__function::__func<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20, std::__1::allocator<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20>, void (std::__1::shared_ptr<yb::rpc::InboundCall>)>::operator()(std::__1::shared_ptr<yb::rpc::InboundCall>&&) ()
#17 0x00005640c51d374f in yb::tserver::PgClientServiceIf::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#18 0x00005640c4f5f420 in yb::rpc::ServicePoolImpl::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#19 0x00005640c4e845af in yb::rpc::InboundCall::InboundCallTask::Run() ()
#20 0x00005640c4f6e243 in yb::rpc::(anonymous namespace)::Worker::Execute() ()
#21 0x00005640c570ecb4 in yb::Thread::SuperviseThread(void*) ()
#22 0x00007f808b7c6694 in start_thread (arg=0x7f76d8caf700) at pthread_create.c:333
#23 0x00007f808bac341d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
```
Jira: DB-12651

Test Plan:
Jenkins
yb_build.sh --cxx-test ql-stress-test QLStressTest.ReproMetaCacheDeadlock

Reviewers: rthallam, hsunder, qhu, timur

Reviewed By: rthallam

Subscribers: ybase, svc_phabricator

Differential Revision: https://phorge.dev.yugabyte.com/D37788
amitanandaiyer added a commit that referenced this issue Sep 6, 2024
@amitanandaiyer
[BACKPORT 2.20][#23747] MetaCache: Callback should not be called whil…
5f40002 
…e holding the lock

Summary:
Original commit: c770d79 / D37706
Call callback in ScopeExit block only. Not while holding the lock.

Without this fix, it is possible that a thread can get into a deadlock, trying to request a shared_lock on a mutex, while already holding an exclusive lock on the same mutex:

This deadlock can be triggered if there are active read/write requests to a Table (from more than 1 thread)  right after the table had a tablet-split.

 If there is only 1 thread, it is unlikely to run into the deadlock, as the thread notices -- as part of the callback -- that the table's partition info is stale. Having a different thread refresh the partition version before the main thread checks if the table version is stale, is likely necessary to trigger the stack trace seen below.

e.g:
```
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x00005640c3eb441b in std::__1::shared_timed_mutex::lock_shared() ()
#2  0x00005640c3ffcbff in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>, yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
#3  0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#4  0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#5  0x00005640c401aa76 in yb::client::(anonymous namespace)::BatcherFlushDone(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig) ()
#6  0x00005640c401b371 in boost::detail::function::void_function_obj_invoker1<std::__1::__bind<void (*)(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig), std::__1::shared_ptr<yb::client::internal::Batcher> const&, std::__1::placeholders::__ph<1> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig&>, void, yb::Status const&>::invoke(boost::detail::function::function_buffer&, yb::Status const&) ()
#7  0x00005640c3f70398 in yb::client::internal::Batcher::Run() ()
#8  0x00005640c3f72656 in yb::client::internal::Batcher::FlushFinished() ()
#9  0x00005640c3f74a4d in yb::client::internal::Batcher::TabletLookupFinished(yb::client::internal::InFlightOp*, yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> >) ()
#10 0x00005640c3f759bc in std::__1::__function::__func<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0, std::__1::allocator<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0>, void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>::operator()(yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&) ()

#11 0x00005640c3fff05d in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>,  yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
** Is holding an exclusive lock in MetaCache::LookupTabletByKey/DoLookupTabletByKey **

#12 0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#13 0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#14 0x00005640c4017130 in yb::client::YBSession::FlushAsync(boost::function<void (yb::client::FlushStatus*)>) ()
#15 0x00005640c5225a0c in yb::tserver::PgClientServiceImpl::Perform(yb::tserver::PgPerformRequestPB const*, yb::tserver::PgPerformResponsePB*, yb::rpc::RpcContext) ()
#16 0x00005640c51c4487 in std::__1::__function::__func<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20, std::__1::allocator<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20>, void (std::__1::shared_ptr<yb::rpc::InboundCall>)>::operator()(std::__1::shared_ptr<yb::rpc::InboundCall>&&) ()
#17 0x00005640c51d374f in yb::tserver::PgClientServiceIf::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#18 0x00005640c4f5f420 in yb::rpc::ServicePoolImpl::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#19 0x00005640c4e845af in yb::rpc::InboundCall::InboundCallTask::Run() ()
#20 0x00005640c4f6e243 in yb::rpc::(anonymous namespace)::Worker::Execute() ()
#21 0x00005640c570ecb4 in yb::Thread::SuperviseThread(void*) ()
#22 0x00007f808b7c6694 in start_thread (arg=0x7f76d8caf700) at pthread_create.c:333
#23 0x00007f808bac341d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
```
Jira: DB-12651

Test Plan:
Jenkins
yb_build.sh --cxx-test ql-stress-test QLStressTest.ReproMetaCacheDeadlock

Reviewers: rthallam, hsunder, qhu, timur

Reviewed By: rthallam

Subscribers: svc_phabricator, ybase

Differential Revision: https://phorge.dev.yugabyte.com/D37789
amitanandaiyer added a commit that referenced this issue Sep 6, 2024
@amitanandaiyer
[BACKPORT 2.20.2][#23747] MetaCache: Callback should not be called wh…
84f9ea7 
…ile holding the lock

Summary:
Original commit: c770d79 / D37706
Call callback in ScopeExit block only. Not while holding the lock.

Without this fix, it is possible that a thread can get into a deadlock, trying to request a shared_lock on a mutex, while already holding an exclusive lock on the same mutex:

This deadlock can be triggered if there are active read/write requests to a Table (from more than 1 thread)  right after the table had a tablet-split.

 If there is only 1 thread, it is unlikely to run into the deadlock, as the thread notices -- as part of the callback -- that the table's partition info is stale. Having a different thread refresh the partition version before the main thread checks if the table version is stale, is likely necessary to trigger the stack trace seen below.

e.g:
```
#0  pthread_cond_wait@@GLIBC_2.3.2 () at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1  0x00005640c3eb441b in std::__1::shared_timed_mutex::lock_shared() ()
#2  0x00005640c3ffcbff in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>, yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
#3  0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#4  0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#5  0x00005640c401aa76 in yb::client::(anonymous namespace)::BatcherFlushDone(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig) ()
#6  0x00005640c401b371 in boost::detail::function::void_function_obj_invoker1<std::__1::__bind<void (*)(std::__1::shared_ptr<yb::client::internal::Batcher> const&, yb::Status const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig), std::__1::shared_ptr<yb::client::internal::Batcher> const&, std::__1::placeholders::__ph<1> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig&>, void, yb::Status const&>::invoke(boost::detail::function::function_buffer&, yb::Status const&) ()
#7  0x00005640c3f70398 in yb::client::internal::Batcher::Run() ()
#8  0x00005640c3f72656 in yb::client::internal::Batcher::FlushFinished() ()
#9  0x00005640c3f74a4d in yb::client::internal::Batcher::TabletLookupFinished(yb::client::internal::InFlightOp*, yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> >) ()
#10 0x00005640c3f759bc in std::__1::__function::__func<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0, std::__1::allocator<yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*)::$_0>, void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>::operator()(yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&) ()

#11 0x00005640c3fff05d in yb::client::internal::MetaCache::LookupTabletByKey(std::__1::shared_ptr<yb::client::YBTable> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::chrono::time_point<yb::CoarseMonoClock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >, std::__1::function<void (yb::Result<scoped_refptr<yb::client::internal::RemoteTablet> > const&)>,  yb::StronglyTypedBool<yb::client::internal::FailOnPartitionListRefreshed_Tag>) ()
** Is holding an exclusive lock in MetaCache::LookupTabletByKey/DoLookupTabletByKey **

#12 0x00005640c3f7549a in yb::client::internal::Batcher::LookupTabletFor(yb::client::internal::InFlightOp*) ()
#13 0x00005640c401855e in yb::client::(anonymous namespace)::FlushBatcherAsync(std::__1::shared_ptr<yb::client::internal::Batcher> const&, boost::function<void (yb::client::FlushStatus*)>, yb::client::YBSession::BatcherConfig, yb::StronglyTypedBool<yb::client::internal::IsWithinTransactionRetry_Tag>) ()
#14 0x00005640c4017130 in yb::client::YBSession::FlushAsync(boost::function<void (yb::client::FlushStatus*)>) ()
#15 0x00005640c5225a0c in yb::tserver::PgClientServiceImpl::Perform(yb::tserver::PgPerformRequestPB const*, yb::tserver::PgPerformResponsePB*, yb::rpc::RpcContext) ()
#16 0x00005640c51c4487 in std::__1::__function::__func<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20, std::__1::allocator<yb::tserver::PgClientServiceIf::InitMethods(scoped_refptr<yb::MetricEntity> const&)::$_20>, void (std::__1::shared_ptr<yb::rpc::InboundCall>)>::operator()(std::__1::shared_ptr<yb::rpc::InboundCall>&&) ()
#17 0x00005640c51d374f in yb::tserver::PgClientServiceIf::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#18 0x00005640c4f5f420 in yb::rpc::ServicePoolImpl::Handle(std::__1::shared_ptr<yb::rpc::InboundCall>) ()
#19 0x00005640c4e845af in yb::rpc::InboundCall::InboundCallTask::Run() ()
#20 0x00005640c4f6e243 in yb::rpc::(anonymous namespace)::Worker::Execute() ()
#21 0x00005640c570ecb4 in yb::Thread::SuperviseThread(void*) ()
#22 0x00007f808b7c6694 in start_thread (arg=0x7f76d8caf700) at pthread_create.c:333
#23 0x00007f808bac341d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
```
Jira: DB-12651

Test Plan:
Jenkins
yb_build.sh --cxx-test ql-stress-test QLStressTest.ReproMetaCacheDeadlock

Reviewers: rthallam, hsunder, qhu, timur

Reviewed By: rthallam

Subscribers: ybase, svc_phabricator

Differential Revision: https://phorge.dev.yugabyte.com/D37831
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ttyusupov ttyusupov

Labels
kind/enhancement This is an enhancement of an existing feature
Projects
None yet
Milestone
v1.0
Development

No branches or pull requests
3 participants
@mbautin @ttyusupov @rkarthik007