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

Add support for Foregin keys table in Pstress #92

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rahulmalik87
Copy link
Contributor

@rahulmalik87 rahulmalik87 commented Aug 2, 2023

Added new table type in pstress with suffix _fk table.
Number of tables depends on fk-prob.
Such table will have column ifk_col reference to a parent table ipkey

for example if --fk-prob is 50 and --tables 100.
there is 50% chance there will be a table tt_N_fk whose parent would be tt_N

Since they linked to pk of parent table. To test FK use --fk-prob=100 --pk-prob=100

To disable foreign keys use --no-fk

Also rename some existing options like

primary-key-probability is now pk-prob
ratio-normal-temp is now temp-prob

Fixed the code around START TRANSACTION. Previously it was running as single transaction which would block transaction on other session.

Decrease the probability of using SAVE POINT in a transaction to 10 % from 25% default

Added new table type in pstress with suffix _fk table.
Number of tables depends on  fk-prob.
Such table will have column ifk_col reference to a parent table ipkey

for example if --fk-prob is 50 and --tables 100.
there is 50% chance there will be a table tt_N_fk whose parent would be tt_N

Since they linked to pk of parent table. To test FK  use --fk-prob=100 --pk-prob=100

To disable foreign keys use --no-fk

Also rename some existing options like

primary-key-probability is now pk-prob
ratio-normal-temp is not temp-prob

Fixed the code around START TRANSACTION. Previously it was running as single transaction which would block transaction on other session.

Decrease the probability of using SAVE POINT in a transaction to 10 % from 25% default

Decrease the probablity of partition tables to 10%;
Decrease the probablity of temporary tables to 10%;
@mohitj1988
Copy link
Contributor

Hi @rahulmalik87 - Thanks for the pull request. We are looking into it.

src/common.hpp Outdated
@@ -120,7 +117,7 @@ struct Option {
MYSQLD_SERVER_OPTION = 'z',
TRANSATION_PRB_K,
TRANSACTIONS_SIZE,
COMMMIT_TO_ROLLBACK_RATIO,
COMMMIT_PROB,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the name to "COMMIT_PROB"

@mohitj1988
Copy link
Contributor

@rahulmalik87 - Few observations

| tt_10_fk | CREATE TABLE tt_10_fk(ifk_colint DEFAULT NULL,ipkeyint NOT NULL AUTO_INCREMENT,c1char(24) DEFAULT NULL,v2_renamevarchar(25) DEFAULT NULL,i3int DEFAULT NULL,v5varchar(10) DEFAULT NULL,tN20_renametinyint(1) DEFAULT NULL,gN277 varchar(13) GENERATED ALWAYS AS (concat(substr(v5,1,6),substr(v5,1,2),substr(i3,1,5))) VIRTUAL, iN24int DEFAULT NULL,tN228tinyint(1) DEFAULT NULL,vN179 varchar(24) DEFAULT NULL, PRIMARY KEY (ipkey), KEY tt_10_fk14 (ipkey,v5 DESC,c1,v2_renameDESC), KEYtt_10_fk783 (tN20_rename,c1,gN277), KEY tt_10_fk750_rename (tN20_rename,v5,ipkey,gN277,c1), KEY tt_10_fk389 (i3,gN277DESC), KEYtt_10_fk656 (tN20_rename DESC,v5 DESC,gN277,ipkey,i3,ifk_col) ) /*!50100 TABLESPACE tab16k_e */ ENGINE=InnoDB AUTO_INCREMENT=9959 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci COMPRESSION='none' /*!80016 ENCRYPTION='Y' */ |

Tested the patch, but don't see which table the foreign key column is referencing to. Normally a foreign key syntax looks like this

FOREIGN KEY (col1) REFERENCES tt_2(col2)

@rahulmalik87
Copy link
Contributor Author

Thank you for testing.
Could you help me to replicate the issue, what is the command line option you used?
I tried pstress-ms --socket $SOCKET --tables 10 -fk-prob=100 --jldd
and the fk_table has the
CONSTRAINT tt_5_fk_tt_5 FOREIGN KEY (ifk_col) REFERENCES tt_5 (ipkey) ON DELETE RESTRICT ON UPDATE CASCADE

@mohitj1988
Copy link
Contributor

mohitj1988 commented Aug 24, 2023

@rahulmalik87 - Use --tables 10 --records 100 --fk-prob 100 --pk-prob 100 --seconds 10

@mohitj1988
Copy link
Contributor

mohitj1988 commented Aug 25, 2023

@rahulmalik87 Some more problems found:

`Test Case1:

./pstress-ps --tables 50 --records 100 --no-temp-tables --no-partition-tables --fk-prob 100 --socket /tmp/mysql_22000.sock --threads 5 --logdir pwd/log

Thread 2 failed, check logs for detail message
Thread 0 failed, check logs for detail message
Thread 1 failed, check logs for detail message
Thread 3 failed, check logs for detail message
Thread 4 failed, check logs for detail message

Error:
F ALTER TABLE tt_12_fk ADD CONSTRAINT tt_12_fk_tt_12 FOREIGN KEY (ifk_col) REFERENCES tt_12 (ipkey) ON UPDATE SET DEFAULT ON DELETE SET DEFAULT
Error Cannot add or update a child row: a foreign key constraint fails (test.#sql-3fe171_1e, CONSTRAINT tt_12_fk_tt_12 FOREIGN KEY (ifk_col) REFERENCES tt_12 (ipkey))
Failed to add fk constraint on tt_12_fk

Test Case 2:

./pstress-ps --tables 10 --records 100 --only-partition-tables --fk-prob 100 --socket /tmp/mysql_22000.sock --threads 5 --logdir pwd/log --jlddl

This creates only one table instead of creating 10 partition tables with FK references

Test Case 3:

./pstress-ps --tables 10 --records 100 --fk-prob 100 --pk-prob 100 --seconds 10 --threads 5 --socket /tmp/mysql_22000.sock --logdir pwd/log

This does not create FK reference tables`

@rahulmalik87
Copy link
Contributor Author

I have fixed the Test case 1.

Fk table tries to insert entries from previous table
and gets an error Failed to add constraint during alter

Now if parent table has 0 records, then fk table will also be empty
@satya-bodapati
Copy link

@rahulmalik87 Thanks for the contribution, can u please look into the other failures?

@satya-bodapati
Copy link

Thank you @rahulmalik87 for reviving/addressing review comments!

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

Successfully merging this pull request may close these issues.

3 participants