Skip to content

Fix Iceberg metadata listing failure when materialized view dropped#10844

Merged
findepi merged 6 commits intotrinodb:masterfrom
findepi:findepi/concurrent-metadata
Mar 4, 2022
Merged

Fix Iceberg metadata listing failure when materialized view dropped#10844
findepi merged 6 commits intotrinodb:masterfrom
findepi:findepi/concurrent-metadata

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Jan 28, 2022 •
edited
Loading

Fix SHOW TABLES and information_schema.tables read failure when
materialized view is concurrently dropped

Fixes #10976

@cla-bot cla-bot bot added the cla-signed label Jan 28, 2022
@findepi findepi force-pushed the findepi/concurrent-metadata branch 2 times, most recently from 37650c4 to dfe8e50 Compare January 28, 2022 16:58
@findepi findepi changed the title Test concurrent metadata access Fix Iceberg metadata listing failure when materialized view dropped Jan 28, 2022
@findepi findepi force-pushed the findepi/concurrent-metadata branch from dfe8e50 to a609f99 Compare January 28, 2022 17:10
@findepi findepi force-pushed the findepi/concurrent-metadata branch from a609f99 to 204cdff Compare January 28, 2022 17:22
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Jan 28, 2022

stress test green https://github.com/trinodb/trino/runs/4984192609?check_suite_focus=true except #9658
dropping stress test commit

@findepi findepi force-pushed the findepi/concurrent-metadata branch from 2bee038 to 6b944ed Compare January 28, 2022 20:04
alexjo2144
alexjo2144 reviewed Jan 28, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@alexjo2144 alexjo2144 left a comment

Choose a reason for hiding this comment

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

Neat test

@findepi findepi force-pushed the findepi/concurrent-metadata branch from c98bfeb to 8e0fcc1 Compare January 31, 2022 16:36
@findepi findepi requested review from kokosing and sopel39 February 1, 2022 08:22
@findepi findepi force-pushed the findepi/concurrent-metadata branch from 8e0fcc1 to 2259982 Compare February 1, 2022 08:23
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Feb 1, 2022

(rebased)

@findepi findepi force-pushed the findepi/concurrent-metadata branch from 2259982 to 906ba83 Compare February 1, 2022 08:24
@sopel39 sopel39 requested a review from raunaqmorarka February 1, 2022 15:43
@findepi findepi mentioned this pull request Feb 2, 2022
Merged
@findepi findepi force-pushed the findepi/concurrent-metadata branch from 906ba83 to 62bc793 Compare February 2, 2022 08:39
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Feb 2, 2022

(rebased)

@findepi findepi force-pushed the findepi/concurrent-metadata branch from 62bc793 to 74c1ec9 Compare February 2, 2022 08:57
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Feb 14, 2022

resolved (snd squashed fixups) to resolve conflicts

@findepi findepi force-pushed the findepi/concurrent-metadata branch 4 times, most recently from 4649ac4 to 9277446 Compare February 21, 2022 09:28
@hashhar hashhar mentioned this pull request Feb 22, 2022
Merged
@findepi findepi force-pushed the findepi/concurrent-metadata branch 2 times, most recently from 8501c7b to 94ebab1 Compare February 25, 2022 15:10
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Feb 25, 2022 •
edited
Loading

rebased & updated after #11134

@findepi findepi force-pushed the findepi/concurrent-metadata branch from bc982fc to f9673ff Compare February 25, 2022 21:39
@findepi findepi marked this pull request as ready for review February 25, 2022 21:39
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Feb 25, 2022

Pin-pointing the problem took me a bit, but it seems it's hopefully all good now.
Removed all the TMP commits and rebased.

@hashhar @losipiuk let me know if you want to take another look, or i should just merge it.

@findepi
Copy link
Copy Markdown
Member Author

findepi commented Feb 26, 2022 

58 successful and 2 failing checks
ci / test (:trino-kudu) (pull_request) Failing after 34m — test (:trino-kudu)
ci / test (:trino-phoenix,:trino-phoenix5) (pull_request) Failing after 23m — test (:trino-phoenix,:trino-phoenix5)

will disable

@findepi findepi force-pushed the findepi/concurrent-metadata branch from f9673ff to dbf8a6f Compare February 26, 2022 16:42
@ebyhr ebyhr mentioned this pull request Feb 28, 2022
Merged
hashhar
hashhar approved these changes Feb 28, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

LGTM % someone more familiar with Iceberg to look at "Fix Iceberg metadata listing failure when materialized view dropped". The fix looks ok to me but I don't know all failure modes in Iceberg connector. cc: @alexjo2144

@findepi
Copy link
Copy Markdown
Member Author

findepi commented Mar 1, 2022

Conflict, will rebase to resolve.

findepi added 5 commits March 1, 2022 16:39
@findepi
Format test name in output consistently
fe6acda
@findepi
Increase log verbosity in table_comments
f2efd0e 
Unexpected exceptions shouldn't be swallowed silently, if they are not
being dealt with.
@findepi
Test concurrent metadata access
3604208 
E.g. `SHOW TABLES` must not fail when tables, views or materialized
views are concurrently created or dropped.
@findepi
Fix Iceberg metadata listing failure when materialized view dropped
b1c961f 
Fix `SHOW TABLES` and `information_schema.tables` read failure when
materialized view is concurrently dropped
@findepi
Move updateColumnComment next to updateTableComment
a396163
@findepi findepi force-pushed the findepi/concurrent-metadata branch from dbf8a6f to a396163 Compare March 1, 2022 15:49
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Mar 4, 2022

CI ci / test (plugin/trino-hive, test-failure-recovery) - #11196 / #10631

@findepi findepi merged commit c55bc4f into trinodb:master Mar 4, 2022
@findepi findepi deleted the findepi/concurrent-metadata branch March 4, 2022 13:23
@findepi findepi mentioned this pull request Mar 4, 2022
Closed
@github-actions github-actions bot added this to the 373 milestone Mar 4, 2022
@mosabua mosabua mentioned this pull request Mar 4, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hashhar hashhar hashhar approved these changes

@losipiuk losipiuk Awaiting requested review from losipiuk

+1 more reviewer

@alexjo2144 alexjo2144 alexjo2144 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed

Milestone

373

Development

Successfully merging this pull request may close these issues.

Failure when listing tables in Iceberg when table/view removed concurrently

3 participants

@findepi @hashhar @alexjo2144