Skip to content

[improvement](fe) Defer external catalog resource cleanup from replay - #69665

Merged
suxiaogang223 merged 4 commits into
apache:masterfrom
suxiaogang223:codex/external-catalog-async-cleanup-pr
Oct 28, 2026
Merged

suxiaogang223 merged 4 commits into
apache:masterfrom
suxiaogang223:codex/external-catalog-async-cleanup-pr

Conversation

@suxiaogang223

@suxiaogang223 suxiaogang223 commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

ALTER CATALOG properties, DROP CATALOG, and catalog rename can block the serial journal replay thread while closing old connectors, contexts, thread pools, access controllers, or Iceberg FileIO resources. A slow close delays subsequent journal entries, including entries for unrelated catalogs.

Apply metadata changes and cache invalidation synchronously, capture the retired instances, and close them through an FE resource reclaimer. The reclaimer retains ownership before dispatch, handles rejected scheduling without running cleanup on the caller, and exposes backlog, running, failure, age, and duration metrics. Iceberg FileIO cleanup runs after the last table reference is released and retains the catalog lease until cleanup finishes.

Bind connector cache retirement to the owning context, including lazy siblings and late registration, so checkpoint and serving Envs with matching catalog IDs cannot retire each other's connector caches. Clean up cache registrations and filesystem resources when root connector construction fails. Journal formats and propagation remain unchanged.

DROP and rename unpublish the old name/ID mappings before controller detachment. They wait for the catalog monitor outside the manager write lock, then try the manager lock and revalidate identity. A busy manager causes the monitor to be released before waiting and retrying, preserving compatibility with manager-first mutation paths. Controller creation, fallback publication, identity checks, and rollback use a captured catalog name so an in-flight publication cannot follow the reused object across rename. Physical cleanup still captures only retired instances.

Scope: catalog initialization under the object lock, cache traversal, quota deregistration, and row-count invalidation remain synchronous. The pending cleanup registry has no hard capacity limit, and this change does not introduce a universal query lease protocol.

The replay tests cover ALTER properties, DROP followed by same-name recreation, and rename in two forms: direct replay calls and serialized JournalEntity entries decoded and applied through EditLog.loadJournal on one serial caller. While the old connector close is held by a latch, the next journal entry must complete and retired cache reads must fail. Replacement metadata is read both before and after old cleanup; the FE name cache is invalidated before the second read so cached results cannot conceal a closed replacement runtime. Only catalog construction is stubbed for CREATE; registration uses the real CatalogMgr path.

This is same-process replay coverage suitable for the FE UT runner. It does not validate cross-FE BDB replication. No Docker cluster, regression framework change, or documentation file is included.

Release note

Reduce journal replay delays caused by closing retired external catalog resources while preserving synchronous metadata and cache invalidation. Prevent orphaned authorization controllers during removal and keep unrelated catalog mutations responsive during slow initialization.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test
    • No need to test or manual test

    Validation on Fedora with JDK 17 and the published third-party prebuilt:

    • Current lifecycle follow-up: bash run-fe-ut.sh --run CatalogRemovalConcurrencyTest,CatalogMgrTest,CatalogVarbinaryMigrationTest,ExternalCatalogReplayCleanupTest,ExternalCatalogDeadlockTest,AccessControllerManagerTest passed all 49 tests across 6 classes (0 failures, errors, or skips; Maven build cache disabled). The new class has 13 cases covering interactive/replay DROP and rename, controller recreation during retirement, custom/fallback publication across rename, replacement isolation, slow initialization, and the contended-lock retry path.
    • Countercheck: the first 11 new cases were run against the original CatalogMgr and AccessControllerManager. Ten failed at the intended assertions/timeouts (4 recreation, 2 rename publication, 4 initialization); the monitor-contention case passed. The final suite also covers both fallback publication variants.
    • Earlier implementation baseline: FE build and packaging passed for all 74 selected modules using build.sh --fe, with UI, Hive UDF and BE Java extension packaging disabled. The affected UT suite passed 113 tests across 10 classes before this follow-up.
    • Final FE Checkstyle passed with 0 violations; git diff --check passed. Cluster regression and cross-FE replication were not run locally.
  • Behavior changed:

    • No.
    • Yes. Retired resources close asynchronously after synchronous metadata and cache invalidation.
  • Does this need documentation?

    • No.
    • Yes.
Lifecycle follow-up: self-review
Checkpoint Conclusion
Goal and evidence Old controllers cannot survive DROP/rename publication races; unrelated mutations progress during blocked initialization. New latch cases reproduce the original failures and pass with the fixes.
Scope Three production Java files and one UT class in this follow-up. A shared removal guard serves interactive and replay DROP/rename.
Concurrency and protection Concurrent queries, authorization publication, mutations/replay, and reclaimer workers are covered. Maps remain concurrent; publication keeps its existing manager-local synchronization.
Lock cost Waiting for the catalog initialization monitor happens without the global manager lock. Existing synchronous retirement and journal writes remain synchronous; physical close remains asynchronous.
Lock order Catalog-first acquisition uses only tryLock for the manager. On contention it releases the catalog monitor before joining the manager queue. Identity is revalidated under both locks; a dedicated test checks monitor availability on this retry path.
Lifecycle and initialization Mappings are unpublished before detachment, the old runtime is captured before name reuse, and authorization rollback retains its original name. AutoCloseable removal state preserves post-lock cleanup even on journal exceptions. Test latches are released and executors terminate in finally; no new static initialization.
Configuration None added.
Compatibility No persisted formats or connector SPI changes.
Thrift/protobuf Unchanged.
Parallel paths Interactive/replay DROP and rename, custom/fallback authorization publication, direct/serialized replay, and migration cleanup are covered.
Conditions Null or stale snapshots are rechecked under the manager lock; retry follows actual identity changes or lock contention. Name snapshots are required because rename reuses the catalog object.
Tests 49 targeted UTs pass, including 13 new concurrency cases. Original-code counterchecks fail at the intended points. Cross-FE BDB replication remains untested locally.
Result files No regression result files added or edited. No documentation files included.
Observability tryLock now records the existing monitored write-lock timings. Existing cleanup metrics and logs remain; futures/latches propagate test failures.
Persistence and failover Existing journal opcodes, formats, mutation ordering and replay routes remain. Cleanup adds no durable state. Cross-process failover was not executed locally.
Data writes and crashes No external data writes or transaction semantics changed; cleanup remains process-local.
FE/BE variables None added or transmitted differently.
Performance No remote initialization wait under the removal manager lock; contended retries join the existing lock queue outside the catalog monitor. Same-catalog initialization and local retirement retain their stated limits.
Other issues Capturing the original name also closes the in-flight rename publication gap. No additional issue identified in the final source review.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

### What problem does this PR solve?

Issue Number: N/A

Problem Summary: Slow closes of retired external catalog resources block
serial journal replay. Apply metadata and cache invalidation synchronously,
then reclaim captured old instances in the background. Scope connector
cache retirement to its context and defer Iceberg FileIO cleanup until the
last table reference is released.

### Release note

Reduce journal replay delays caused by closing retired external catalog resources.

### Check List (For Author)

- Test: FE build and Checkstyle passed; 113 unit tests across 10 classes passed.
- Behavior changed: Yes. Retired resources close asynchronously after synchronous invalidation.
- Does this need documentation: No.
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69665

Problem Summary: Cover ALTER, DROP followed by same-name recreation, and rename through journal serialization and the real replay dispatcher. Verify that a blocked old connector close does not stall the next journal entry, that old metadata is retired synchronously, and that replacement metadata remains accessible before and after cleanup without relying on a cached result.

### Release note

None

### Check List (For Author)

- Test: Unit Test via run-fe-ut.sh --run ExternalCatalogReplayCleanupTest (8 passed); FE Checkstyle (0 violations); git diff --check.
- Behavior changed: No. Test coverage only.
- Does this need documentation: No.
@suxiaogang223

Copy link
Copy Markdown
Member Author

run buildall

@suxiaogang223

Copy link
Copy Markdown
Member Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed head def8c0b1be70f02984d3357b94b7b8d36cd951a2. Two P2 lifecycle/concurrency regressions are reported inline: a controller can be republished during DROP/rename and escape cleanup, and DROP now waits for network-bound catalog initialization while holding the global manager lock. No P0/P1 finding or existing blocking comment was identified. There were no existing inline threads and no additional user-provided focus points.

Critical checkpoint conclusions:

  • Goal and evidence: logical cache retirement/property publication remain synchronous and physical resource close moves to the reclaimer. The new latch-based direct/serialized replay tests cover progress past blocked close, but miss the two interleavings reported inline.
  • Scope and clarity: the change is focused on catalog/resource lifecycle, shared cache ownership and cleanup observability; the reusable owner/reclaimer boundaries are coherent.
  • Concurrency and protection: query initialization/loaders/borrowers race with interactive mutations, replay/checkpoint and reclaimer workers. CatalogMgr uses its write lock, initialization/detachment use the catalog monitor, owner registration is synchronized, and cache publication/refcounts fence stale loads. Access-controller publication remains outside the catalog monitor and creates the first finding.
  • Lock cost: physical connector/context/controller/FileIO close is deferred. Local cache retirement remains synchronous. Acquiring the initialization monitor under the global manager lock introduces the second finding.
  • Lock order: no additional concrete lock cycle was established; the verified global-lock wait is contention on remote initialization, not a claimed deadlock.
  • Lifecycle and initialization: obsolete cleanup captures exact instances; failed root construction retires its own caches; lazy siblings share their context owner; checkpoint and serving contexts with matching IDs remain isolated. Iceberg retains the catalog lease through deferred table cleanup and pins the plugin loader. The controller publication gap remains a lifecycle defect. No new cross-translation-unit static initialization applies to this Java change.
  • Configuration: no settings are added; existing property updates retire and rebuild affected runtime/cache state.
  • Compatibility: no persisted schema or public connector SPI change was found. Journal operation types/formats remain unchanged; dropped-object closure is transient.
  • Thrift/protobuf: unchanged; cross-branch field-ID review is not applicable.
  • Parallel paths: interactive and replay ALTER/DROP/rename, CREATE validation/image initialization, failed construction, root/lazy sibling, and disabled/weighted Iceberg cache paths were traced. Master and replay share the rename transition.
  • Conditions: retirement, registration rejection, last-reference release, idempotent close and rejected dispatch guards correspond to concrete lifecycle states. Rejected tasks retain ownership and do not run close on the caller.
  • Tests: new unit tests use latches/recording fakes for blocked cleanup, replacement isolation, late registration, failure accounting, TCCL and borrower dependencies. Serialized JournalEntity/EditLog coverage is same-process; controller republishing during removal and cold initialization during DROP are uncovered cases.
  • Result files: no regression result files were changed; no executed test result is asserted by this review.
  • Observability: pending/running/failure/age/duration gauges and backlog/slow/failure logs cover the new cleanup path; stages attempt later cleanup after a failure.
  • Persistence and failover: existing EditLog write/replay routes are preserved, and cleanup actions carry no new durable state. Cross-FE replication/failover behavior was not executed.
  • Data writes/crashes: this patch changes local resource and metadata lifecycle, not external data writes or transaction visibility. A crash discards process-local cleanup state; no new data-write atomicity protocol is introduced.
  • FE/BE variables: none added or transmitted differently. BE visible versions, delete bitmaps, nullable columns and MemTracker accounting are outside the changed paths.
  • Performance: queue draining avoids rescanning pending work per short close. Retirement cost and oldest-age scans scale with entries/backlog. Four workers and an unbounded pending registry retain the documented saturation limits; the global-lock amplification is actionable separately.
  • Other issues and user concerns: no additional substantiated issue after the final sweep. All changed files and initial risk items have explicit dispositions.

Static inspection only: no builds, tests, benchmarks, source modifications or runtime validation were performed. External SDK close behavior and cross-FE replication remain validation limits, not unresolved review candidates. Review converged after two bounded rounds.

Review converged after 2 round(s).

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28424 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit def8c0b1be70f02984d3357b94b7b8d36cd951a2, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17609	5123	5070	5070
q2	2156	293	253	253
q3	10261	1387	822	822
q4	4680	461	334	334
q5	7434	795	536	536
q6	182	189	153	153
q7	758	804	596	596
q8	9267	1394	1085	1085
q9	5604	4172	4181	4172
q10	6863	1294	1036	1036
q11	425	246	227	227
q12	641	396	276	276
q13	18109	3687	3046	3046
q14	280	266	237	237
q15	q16	726	723	645	645
q17	1218	751	583	583
q18	7053	6198	6152	6152
q19	1109	961	614	614
q20	386	340	224	224
q21	5210	2102	2263	2102
q22	355	304	261	261
Total cold run time: 100326 ms
Total hot run time: 28424 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5609	5589	5594	5589
q2	694	586	532	532
q3	5228	5396	4936	4936
q4	1988	2137	1533	1533
q5	5271	5140	5171	5140
q6	234	193	141	141
q7	2058	1897	1697	1697
q8	2854	2484	2475	2475
q9	7555	7549	7533	7533
q10	3638	3591	3166	3166
q11	598	413	387	387
q12	662	668	504	504
q13	3291	3575	3069	3069
q14	307	294	263	263
q15	q16	658	707	620	620
q17	7792	7066	6864	6864
q18	12956	12159	12934	12159
q19	904	800	780	780
q20	2152	2147	1928	1928
q21	5606	4601	4659	4601
q22	501	448	409	409
Total cold run time: 70556 ms
Total hot run time: 64326 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151005 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit def8c0b1be70f02984d3357b94b7b8d36cd951a2, data reload: false

query5	4309	585	446	446
query6	437	207	187	187
query7	4793	441	216	216
query8	321	166	166	166
query9	8768	3118	3116	3116
query10	476	290	258	258
query11	5980	3846	3561	3561
query12	139	96	84	84
query13	1256	456	330	330
query14	6156	3959	3658	3658
query14_1	3423	3403	3447	3403
query15	208	195	179	179
query16	959	466	426	426
query17	862	664	554	554
query18	2428	431	311	311
query19	196	156	143	143
query20	86	86	83	83
query21	215	127	108	108
query22	12921	12929	12764	12764
query23	13622	13219	12594	12594
query23_1	12533	12561	12568	12561
query24	7412	826	472	472
query24_1	477	479	474	474
query25	539	432	374	374
query26	1272	261	140	140
query27	2756	449	291	291
query28	4625	1969	1946	1946
query29	1585	593	450	450
query30	298	232	187	187
query31	1006	924	815	815
query32	138	90	94	90
query33	537	307	248	248
query34	938	852	491	491
query35	721	748	650	650
query36	819	826	766	766
query37	119	100	81	81
query38	1773	1745	1665	1665
query39	794	796	740	740
query39_1	734	737	743	737
query40	219	117	99	99
query41	73	68	73	68
query42	89	85	87	85
query43	344	366	314	314
query44	1285	696	716	696
query45	179	179	170	170
query46	818	964	559	559
query47	2862	2952	2565	2565
query48	333	295	215	215
query49	564	384	284	284
query50	672	269	208	208
query51	10317	10358	10523	10358
query52	80	83	67	67
query53	185	206	150	150
query54	243	209	201	201
query55	73	69	65	65
query56	226	217	222	217
query57	1638	1593	1559	1559
query58	290	267	253	253
query59	2193	2210	2015	2015
query60	302	243	226	226
query61	149	149	147	147
query62	386	325	283	283
query63	183	154	157	154
query64	2742	968	765	765
query65	3335	3259	3250	3250
query66	1771	410	315	315
query67	20233	20276	20062	20062
query68	3245	996	608	608
query69	392	280	262	262
query70	848	783	761	761
query71	308	233	211	211
query72	2833	2659	2279	2279
query73	528	519	290	290
query74	4628	4485	4324	4324
query75	2353	2289	1895	1895
query76	2399	1008	593	593
query77	340	363	271	271
query78	9860	9728	9281	9281
query79	986	830	521	521
query80	1151	445	349	349
query81	588	332	269	269
query82	591	138	108	108
query83	309	197	185	185
query84	304	133	104	104
query85	875	438	378	378
query86	374	239	234	234
query87	2012	1976	1795	1795
query88	3654	2696	2660	2660
query89	334	281	242	242
query90	1858	175	174	174
query91	166	154	131	131
query92	102	79	89	79
query93	999	1004	581	581
query94	587	312	278	278
query95	576	400	303	303
query96	675	471	232	232
query97	2481	2437	2329	2329
query98	174	151	148	148
query99	767	760	652	652
Total cold run time: 235505 ms
Total hot run time: 151005 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.31 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit def8c0b1be70f02984d3357b94b7b8d36cd951a2, data reload: false

query1	0.01	0.00	0.00
query2	0.09	0.04	0.04
query3	0.27	0.09	0.08
query4	1.64	0.11	0.10
query5	0.24	0.24	0.24
query6	1.17	0.69	0.68
query7	0.04	0.01	0.01
query8	0.06	0.04	0.03
query9	0.39	0.34	0.32
query10	0.56	0.56	0.54
query11	0.19	0.15	0.15
query12	0.19	0.14	0.16
query13	0.50	0.49	0.48
query14	1.04	1.04	1.01
query15	0.63	0.62	0.61
query16	0.35	0.35	0.32
query17	1.17	1.09	1.15
query18	0.19	0.19	0.19
query19	2.09	1.99	1.93
query20	0.02	0.01	0.01
query21	15.46	0.19	0.14
query22	4.94	0.05	0.05
query23	16.29	0.33	0.12
query24	2.92	0.59	0.60
query25	0.10	0.07	0.05
query26	1.57	0.22	0.15
query27	0.06	0.03	0.05
query28	3.90	0.77	0.34
query29	12.52	4.13	3.24
query30	0.29	0.13	0.10
query31	2.77	0.57	0.32
query32	3.23	0.60	0.51
query33	3.20	3.18	3.20
query34	15.38	3.97	3.37
query35	3.37	3.34	3.36
query36	0.56	0.50	0.46
query37	0.08	0.06	0.07
query38	0.05	0.03	0.04
query39	0.03	0.02	0.02
query40	0.17	0.15	0.14
query41	0.09	0.03	0.03
query42	0.04	0.03	0.03
query43	0.05	0.04	0.04
Total cold run time: 97.91 s
Total hot run time: 24.31 s

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69665

Problem Summary: The catalog migration UT assumes authorization cleanup completes before migration returns. Background cleanup can still be pending, causing FE UT to fail in CI. Await the cleanup with a bounded FutureTask, propagate its assertion failures to the test caller, and check that the catalog write lock is released across all threads when cleanup runs. Journal ordering and migrated properties remain asserted.

### Release note

None

### Check List (For Author)

- Test: Unit Test via run-fe-ut.sh --run CatalogVarbinaryMigrationTest (7 tests); FE Checkstyle; git diff --check.
- Behavior changed: No. Test synchronization only.
- Does this need documentation: No.
@suxiaogang223

Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28567 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit ba0f1a172987b007e85cb48ea66a1e1648972a24, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17613	5266	5100	5100
q2	2114	329	271	271
q3	10218	1404	843	843
q4	4679	471	335	335
q5	7431	801	534	534
q6	177	182	155	155
q7	745	811	592	592
q8	9261	1362	1063	1063
q9	5390	4217	4225	4217
q10	6833	1291	1027	1027
q11	424	258	228	228
q12	641	406	292	292
q13	18106	3675	3023	3023
q14	286	271	250	250
q15	q16	722	721	651	651
q17	1189	753	606	606
q18	6962	6176	6088	6088
q19	1139	958	607	607
q20	383	346	238	238
q21	5529	2185	2458	2185
q22	354	307	262	262
Total cold run time: 100196 ms
Total hot run time: 28567 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5597	5600	5620	5600
q2	679	580	519	519
q3	5213	5382	4946	4946
q4	2009	2144	1555	1555
q5	5275	5158	5736	5158
q6	259	195	153	153
q7	2343	2102	1910	1910
q8	3117	2987	2687	2687
q9	7947	8064	8061	8061
q10	3728	3693	3270	3270
q11	629	459	422	422
q12	687	669	531	531
q13	3609	3812	3263	3263
q14	321	308	294	294
q15	q16	693	703	693	693
q17	8334	7647	7015	7015
q18	12973	12216	12818	12216
q19	912	815	817	815
q20	2133	2141	1933	1933
q21	5596	4608	4715	4608
q22	513	432	413	413
Total cold run time: 72567 ms
Total hot run time: 66062 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150586 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit ba0f1a172987b007e85cb48ea66a1e1648972a24, data reload: false

query5	4311	599	427	427
query6	422	210	194	194
query7	4813	445	225	225
query8	324	161	144	144
query9	8780	3123	3140	3123
query10	495	306	258	258
query11	6012	3872	3581	3581
query12	154	92	88	88
query13	1259	455	327	327
query14	6100	3922	3645	3645
query14_1	3437	3425	3422	3422
query15	205	207	179	179
query16	953	419	399	399
query17	869	697	532	532
query18	2425	430	311	311
query19	209	167	138	138
query20	86	83	83	83
query21	210	128	108	108
query22	12849	13018	12760	12760
query23	13317	12957	12611	12611
query23_1	12542	12580	12728	12580
query24	7350	820	464	464
query24_1	493	486	475	475
query25	532	422	362	362
query26	1267	256	143	143
query27	2764	447	271	271
query28	4623	1949	1933	1933
query29	1570	606	455	455
query30	305	224	187	187
query31	1008	937	812	812
query32	159	94	97	94
query33	548	303	249	249
query34	898	843	496	496
query35	725	771	650	650
query36	817	807	734	734
query37	122	103	88	88
query38	1783	1712	1658	1658
query39	797	803	745	745
query39_1	732	724	733	724
query40	224	120	96	96
query41	75	69	69	69
query42	87	88	88	88
query43	349	358	322	322
query44	1315	679	687	679
query45	175	167	161	161
query46	818	960	570	570
query47	2747	2732	2348	2348
query48	283	317	232	232
query49	592	402	296	296
query50	688	271	211	211
query51	10703	10393	10495	10393
query52	80	81	70	70
query53	186	205	146	146
query54	230	202	184	184
query55	73	70	61	61
query56	254	214	227	214
query57	1686	1601	1555	1555
query58	285	266	259	259
query59	2129	2216	1966	1966
query60	297	240	222	222
query61	145	144	148	144
query62	387	342	276	276
query63	194	159	157	157
query64	2743	925	782	782
query65	3336	3252	3256	3252
query66	1776	407	301	301
query67	20390	20273	19943	19943
query68	3250	1017	622	622
query69	427	286	244	244
query70	855	767	775	767
query71	306	236	207	207
query72	2837	2591	2299	2299
query73	513	538	312	312
query74	4617	4521	4330	4330
query75	2326	2302	1916	1916
query76	2390	1035	612	612
query77	351	371	275	275
query78	9764	9715	9250	9250
query79	984	858	506	506
query80	1213	434	322	322
query81	592	314	265	265
query82	809	137	110	110
query83	319	196	184	184
query84	314	128	97	97
query85	872	444	371	371
query86	399	239	238	238
query87	1992	1964	1810	1810
query88	3767	2691	2673	2673
query89	336	287	239	239
query90	1933	168	162	162
query91	160	148	127	127
query92	102	89	88	88
query93	1147	1015	595	595
query94	652	313	276	276
query95	571	402	292	292
query96	656	503	243	243
query97	2434	2447	2341	2341
query98	160	153	148	148
query99	775	755	640	640
Total cold run time: 236134 ms
Total hot run time: 150586 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.32 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit ba0f1a172987b007e85cb48ea66a1e1648972a24, data reload: false

query1	0.00	0.00	0.01
query2	0.09	0.05	0.05
query3	0.26	0.08	0.08
query4	1.61	0.09	0.11
query5	0.23	0.23	0.24
query6	1.17	0.68	0.69
query7	0.04	0.01	0.01
query8	0.06	0.04	0.03
query9	0.39	0.35	0.36
query10	0.57	0.60	0.57
query11	0.20	0.15	0.14
query12	0.18	0.14	0.15
query13	0.49	0.49	0.49
query14	1.03	1.01	1.04
query15	0.62	0.60	0.60
query16	0.35	0.35	0.35
query17	1.18	1.13	1.14
query18	0.22	0.19	0.20
query19	2.10	1.96	2.00
query20	0.02	0.01	0.01
query21	15.46	0.19	0.13
query22	4.91	0.05	0.05
query23	16.30	0.31	0.12
query24	2.90	0.50	0.37
query25	0.12	0.06	0.06
query26	1.42	0.22	0.16
query27	0.05	0.03	0.04
query28	3.88	0.78	0.39
query29	12.47	4.22	3.25
query30	0.28	0.13	0.11
query31	2.76	0.56	0.32
query32	3.23	0.60	0.50
query33	3.14	3.15	3.24
query34	15.47	4.01	3.42
query35	3.36	3.36	3.36
query36	0.55	0.46	0.44
query37	0.09	0.07	0.06
query38	0.04	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.14	0.14
query41	0.09	0.03	0.02
query42	0.04	0.03	0.03
query43	0.04	0.03	0.03
Total cold run time: 97.62 s
Total hot run time: 24.32 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 23.21% (208/896) 🎉
Increment coverage report
Complete coverage report

### What problem does this PR solve?

Issue Number: N/A

Related PR: apache#69665

Problem Summary: DROP and rename can detach an authorization controller while the old catalog remains published, allowing a concurrent lookup to install a controller that escapes cleanup. In-flight authorization publication can also follow the reused catalog object's new name after rename. Unpublish the mappings before detachment and retain the original name throughout authorization publication and rollback. Removal also waited for network-bound initialization while holding the global catalog lock. Acquire the catalog monitor outside that lock, use a nonblocking manager-lock attempt with identity revalidation, and release the monitor before waiting on manager contention. Preserve synchronous retirement, captured-instance asynchronous close, journal ordering, post-lock cleanup, and monitored write-lock timing.

### Release note

Prevent orphaned authorization controllers during catalog removal and keep unrelated catalog mutations responsive while another catalog initializes.

### Check List (For Author)

- Test: Unit Test via run-fe-ut.sh across CatalogRemovalConcurrencyTest, CatalogMgrTest, CatalogVarbinaryMigrationTest, ExternalCatalogReplayCleanupTest, ExternalCatalogDeadlockTest, and AccessControllerManagerTest (49 tests); final FE Checkstyle (0 violations); git diff --check. Original-code counterchecks reproduced 10 expected failures among the first 11 new cases.
- Behavior changed: Yes. DROP and rename wait for initialization outside the global catalog lock and fence old-name authorization publication.
- Does this need documentation: No.
@suxiaogang223

Copy link
Copy Markdown
Member Author

run buildall

@suxiaogang223

Copy link
Copy Markdown
Member Author

/review Re-check both P2 findings after the lifecycle fix. Pay particular attention to unpublishing mappings before controller detachment, original-name identity checks and rollback for custom/fallback controller publication across rename, catalog-monitor acquisition outside the manager lock, nonblocking manager-lock acquisition and snapshot revalidation, and the 13 latch/lock-contention unit cases. Also check for regressions in old/new runtime isolation and journal ordering.

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: workspace routing discovery unauthorized (401)
Workflow run: https://github.com/apache/doris/actions/runs/40620998736

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@suxiaogang223

Copy link
Copy Markdown
Member Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: workspace routing discovery unauthorized (401)
Workflow run: https://github.com/apache/doris/actions/runs/40621210858

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28324 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit e235389807ca6c7a39a4c8f0682ddf24eec87d35, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17591	5105	5134	5105
q2	2141	330	257	257
q3	10229	1361	815	815
q4	4682	465	336	336
q5	7427	810	534	534
q6	181	184	151	151
q7	756	806	602	602
q8	9252	1367	1063	1063
q9	5442	4170	4177	4170
q10	6818	1306	1017	1017
q11	438	251	225	225
q12	645	402	282	282
q13	18050	3464	2964	2964
q14	265	266	245	245
q15	q16	716	707	659	659
q17	1198	750	587	587
q18	6959	6158	6104	6104
q19	1103	971	590	590
q20	367	326	219	219
q21	4649	2146	2274	2146
q22	351	292	253	253
Total cold run time: 99260 ms
Total hot run time: 28324 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5582	5578	5556	5556
q2	694	580	517	517
q3	5191	5386	4907	4907
q4	1998	2127	1547	1547
q5	5273	5181	5307	5181
q6	265	205	162	162
q7	2357	2031	1885	1885
q8	3013	2775	2675	2675
q9	8055	7973	7953	7953
q10	3707	3884	3207	3207
q11	615	463	415	415
q12	691	691	519	519
q13	3535	3872	3296	3296
q14	301	310	285	285
q15	q16	733	691	649	649
q17	8256	7147	6876	6876
q18	13003	12209	12831	12209
q19	901	765	774	765
q20	2136	2128	1894	1894
q21	5542	4552	4658	4552
q22	528	456	391	391
Total cold run time: 72376 ms
Total hot run time: 65441 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151384 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit e235389807ca6c7a39a4c8f0682ddf24eec87d35, data reload: false

query5	4333	643	445	445
query6	427	207	217	207
query7	4816	462	228	228
query8	326	182	178	178
query9	8793	3289	3272	3272
query10	501	304	258	258
query11	6068	3864	3584	3584
query12	151	92	89	89
query13	1259	444	324	324
query14	6134	3956	3658	3658
query14_1	3457	3492	3432	3432
query15	210	195	191	191
query16	929	426	411	411
query17	869	677	549	549
query18	2412	438	311	311
query19	203	167	139	139
query20	89	86	86	86
query21	211	134	113	113
query22	12897	13100	12854	12854
query23	13382	12749	12488	12488
query23_1	12484	12372	12630	12372
query24	7373	864	479	479
query24_1	510	490	488	488
query25	544	442	378	378
query26	1263	283	147	147
query27	2744	495	273	273
query28	4547	2070	2064	2064
query29	1545	615	426	426
query30	309	224	190	190
query31	998	940	804	804
query32	135	93	84	84
query33	515	304	237	237
query34	900	914	503	503
query35	742	761	632	632
query36	806	822	727	727
query37	126	92	78	78
query38	1772	1734	1654	1654
query39	780	778	740	740
query39_1	727	732	734	732
query40	226	123	98	98
query41	66	64	62	62
query42	87	84	84	84
query43	350	367	312	312
query44	1403	711	732	711
query45	178	180	165	165
query46	886	1008	583	583
query47	2794	2785	2675	2675
query48	293	322	226	226
query49	570	415	292	292
query50	719	294	218	218
query51	10311	10478	10096	10096
query52	80	85	70	70
query53	203	221	160	160
query54	267	206	183	183
query55	74	68	65	65
query56	224	229	221	221
query57	1658	1622	1554	1554
query58	275	259	259	259
query59	2133	2111	1944	1944
query60	297	251	234	234
query61	146	145	153	145
query62	394	340	299	299
query63	193	160	166	160
query64	2744	951	770	770
query65	3320	3276	3264	3264
query66	1772	445	314	314
query67	20267	20100	20002	20002
query68	2943	1027	631	631
query69	396	293	254	254
query70	854	824	783	783
query71	288	229	217	217
query72	2758	2603	2446	2446
query73	541	559	306	306
query74	4628	4486	4388	4388
query75	2469	2340	1973	1973
query76	2308	1133	628	628
query77	360	396	285	285
query78	9757	9657	9122	9122
query79	1024	887	556	556
query80	554	451	363	363
query81	544	330	272	272
query82	577	136	121	121
query83	305	196	186	186
query84	323	128	103	103
query85	810	443	376	376
query86	316	239	231	231
query87	1989	1968	1799	1799
query88	3861	2808	2809	2808
query89	348	290	243	243
query90	1892	184	166	166
query91	157	157	127	127
query92	101	91	108	91
query93	1065	1048	582	582
query94	439	311	271	271
query95	592	331	359	331
query96	701	503	238	238
query97	2452	2442	2322	2322
query98	166	154	153	153
query99	763	758	651	651
Total cold run time: 234779 ms
Total hot run time: 151384 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.31 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit e235389807ca6c7a39a4c8f0682ddf24eec87d35, data reload: false

query1	0.01	0.01	0.00
query2	0.09	0.05	0.05
query3	0.26	0.09	0.09
query4	1.61	0.10	0.10
query5	0.23	0.24	0.23
query6	1.16	0.69	0.68
query7	0.04	0.01	0.00
query8	0.05	0.04	0.03
query9	0.41	0.36	0.35
query10	0.57	0.57	0.58
query11	0.20	0.14	0.13
query12	0.18	0.16	0.16
query13	0.49	0.50	0.48
query14	1.02	0.99	1.01
query15	0.63	0.60	0.61
query16	0.32	0.35	0.34
query17	1.10	1.09	1.12
query18	0.21	0.18	0.21
query19	2.10	1.96	2.00
query20	0.02	0.01	0.02
query21	15.48	0.20	0.14
query22	4.89	0.05	0.06
query23	16.27	0.32	0.12
query24	2.48	0.98	0.43
query25	0.11	0.07	0.04
query26	1.67	0.22	0.16
query27	0.04	0.05	0.03
query28	3.42	0.73	0.36
query29	12.52	4.08	3.25
query30	0.28	0.13	0.10
query31	2.77	0.56	0.32
query32	3.22	0.60	0.50
query33	3.14	3.24	3.23
query34	15.46	4.00	3.40
query35	3.37	3.36	3.34
query36	0.54	0.45	0.44
query37	0.09	0.07	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.15	0.16
query41	0.09	0.03	0.02
query42	0.04	0.03	0.03
query43	0.04	0.03	0.03
Total cold run time: 96.88 s
Total hot run time: 24.31 s

@suxiaogang223

Copy link
Copy Markdown
Member Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed all 26 changed files at e235389. No new actionable finding was verified. Both earlier P2 findings are addressed: unpublish before controller detachment and avoid waiting for initialization under the global manager lock. Neither is reposted; no existing P0/P1 finding remains to carry forward.

The two complete-coverage reviewers and separate lifecycle-risk reviewer each returned NO_NEW_VALUABLE_FINDINGS. All initial risks have evidence-backed dispositions, with no unresolved concrete candidate or uncovered static scope. No additional user review focus was supplied.

Critical checkpoint Conclusion
Goal and supporting evidence Synchronous metadata/cache retirement captures old instances; physical connector, context, controller and FileIO cleanup is dispatched. Added latch tests express replay progress and replacement isolation; their source was inspected, not executed.
Scope and reuse The 14 production and 12 test files form a cohesive lifecycle change, sharing removal, ownership and reclamation helpers.
Concurrent actors and protection Query initialization/loaders, DDL, serial replay and reclaimer workers interact through catalog monitors, manager locks, concurrent maps, publication gates and atomic resource references. No new concrete race was verified.
Work under locks Logical retirement remains synchronous. Removal waits for initialization before acquiring the manager lock; detached physical cleanup does not wait on the replay caller. Existing synchronous initialization/invalidation limits remain.
Lock order Removal uses catalog monitor then nonblocking manager tryLock; on contention it releases the monitor before waiting for the manager. Identity is revalidated. Cache remote loaders run outside publication/key gates, and retirement does not await them. No new wait cycle was established.
Lifecycle and initialization Cleanup closures capture retired instances. Immediate children precede connector/context close; delayed Iceberg FileIO cleanup retains its catalog lease through completion. Plugin classloaders are pinned/restored. No static initialization dependency issue was found.
Cache ownership Root and later sibling construction share the owning context; late registration is rejected and cleaned. Retirement uses exact connector owners and Env-local core containers, avoiding ID-wide connector retirement across checkpoint/serving Envs.
Configuration No configuration item is added.
Compatibility No persisted representation or connector SPI change requires upgrade compatibility code.
Thrift/protobuf No definitions or numeric IDs are changed.
Parallel paths Interactive/replay DROP and rename, ALTER, migration cleanup, custom/fallback controller publication, root/lazy siblings and direct/serialized journal replay were traced.
Conditional checks Identity, closed-state, retirement and contention checks correspond to reachable lifecycle races; no unsupported defensive branch was identified.
Test coverage JUnit/latch cases cover slow close/initialization, same-name recreation, rename publication, rejection, cleanup failure, late registration and borrower ordering. Same-process journal dispatch is covered in source; cross-FE replication and deployed-plugin behavior were not validated here.
Result files No regression expected-output file is added or modified.
Observability Pending/running/failure counts, oldest age, duration and slow/backlog/failure logging expose asynchronous reclamation.
Persistence and failover Existing CatalogLog formats and EditLog routing remain; mutation/write and replay paths match. Marker migration still journals first and schedules cleanup after the outer lock. Reclamation adds no durable state.
Data writes and crashes No external data-write, transaction visibility, partition-version or delete-bitmap semantics change. Cleanup ownership is process-local; a failed close attempt is logged/counted and is not implicitly retried.
FE/BE variables No new transmitted variable or serialization path is introduced.
Performance and resources Queue polling avoids repeated pending-set rescans during drain; saturation retains tasks without caller-runs cleanup. Backlog has no hard capacity bound, as documented by the PR. No additional concrete performance regression was verified.
Other issues Further rechecks covered late loads, resource dependency ordering, failed construction, replacement cleanup and rename/controller schedules; none produced another substantiated finding.

This is a static review only: no builds, tests, benchmarks, cluster runs or cross-FE replication checks were executed. The implementation retains the stated same-catalog initialization/cache-traversal limits and does not provide a universal query lease protocol. Review converged in one round.

Review converged after 1 round(s).

@suxiaogang223
suxiaogang223 merged commit 0827361 into apache:master Oct 28, 2026
42 of 43 checks passed
@suxiaogang223
suxiaogang223 deleted the codex/external-catalog-async-cleanup-pr branch October 28, 2026 12:59
morningman added a commit that referenced this pull request Oct 31, 2026
…ing it finish (#69874)

### What problem does this PR solve?

Issue Number: None

Related PR: #66331, #68353

Problem Summary:

**In short:** dropping or altering an ADBC catalog can crash the FE
process. Closing the catalog frees the native ADBC objects at once,
including the connections and results that other threads are still
reading, such as the column-statistics loads that a query on the catalog
has just started. This kills the FE in about one External regression run
in 75, whatever the PR under test changes. With this PR, closing a
client only refuses new calls, and the last call still in flight
releases the driver as it leaves. DROP and ALTER CATALOG still return
without waiting.

This commit was first pushed as the last commit of #68353, which is
still in review. It shares nothing with the rest of that PR, so it is
carved out here to land on its own.

#### Background

An ADBC catalog (#66331) reads its metadata through `AdbcClient`, which
owns one native `AdbcDatabase` and one Arrow allocator per catalog.
Every metadata call borrows a fresh connection through
`AdbcClient.withConnection` and closes it afterwards. FE talks to the C
driver through adbc-driver-jni 0.24. In that bridge, closing a parent
closes everything still open under it, on the closing thread:
`JniDatabase.close()` closes every connection still open on the
database, and `JniConnection.close()` closes every result (`getObjects`
and similar readers) still open on the connection. The bridge only
checks a handle before a call starts, and the C driver does not check at
all, so freeing an object that another thread is still using is a
use-after-free in native code, which takes the whole JVM down rather
than failing one call.

Column statistics reach the driver too. When a query plans against an
ADBC table, the STATS_FETCH pool loads column statistics in the
background. A load that finds nothing in the statistics table asks the
table itself: `PluginDrivenExternalTable.getColumnStatistic` →
`AdbcConnectorMetadata.getTableHandle` → `withConnection` →
`getObjects`. With `"meta.cache.adbc.metadata.enable" = "false"`, every
such load calls into the driver.

#### The problem, and what it cost

- `AdbcClient.close()` closed the database and then the allocator
immediately, whatever was still in flight. DROP CATALOG and ALTER
CATALOG ... SET PROPERTIES close the catalog's connector this way, on
their own thread (since #69665, on the `ExternalResourceReclaimer` pool;
before that, synchronously on the statement thread, with the same race).
- A statistics load that was reading a `getObjects` result, or was
inside the driver on its connection, then read memory that had just been
freed. Where it dies depends on where that thread is at that moment.
- In CI, the External regression pipeline lost its FE in 11 of the 834
runs between 2026-10-22 and 2026-10-30. Each time, 54 to 56 suites then
failed with `Communications link failure`, so the PR under test showed a
red run it did not cause. All 11 died at the same moment:
`test_adbc_sqlite_catalog_scan` drops its catalog with the metadata
cache turned off right after querying it. In the same millisecond,
fe.log shows the allocator closing with `Memory was leaked by query` and
a statistics load failing with `Failed to read the ADBC getObjects
result: ArrowArrayStream is already closed`. That is the same race,
caught by a thread that came back only after the close had finished.

| FE crash in `fe.out` | Runs |
| --- | --- |
| `SIGSEGV` in
`nanoarrow::ArrayStreamFactory<VectorArrayStream>::get_next_wrapper+0x4`
| 6 |
| `SIGSEGV` in `PrivateArrowSchemaDeepCopy+0x46` | 2 |
| glibc `double free or corruption (fasttop)` | 2 |
| no message | 1 |
| `SIGSEGV` in `sqlite3FindTable` (an earlier run, 2026-10-06) | 1 |

- Users can hit this too, not only CI: DROP or ALTER an ADBC catalog
while a statement or a statistics load is still reading it.

#### How this PR fixes it

- `AdbcClient` counts the calls inside `withConnection` (`enter` /
`leave`, under the client's monitor, which also opens the database on
first use).
- `close()` marks the client closed, so new calls are refused with
`AdbcClient has been closed`. It releases the database and the allocator
itself only when no call is in flight. Otherwise it returns at once, and
the last call to leave releases them.
- A deferred release that fails is logged with the driver's path, as the
catalog already logs a failed connector close. It is not thrown at that
last caller, whose own work succeeded and which did not close anything.
- `close()` never waits for the calls in flight, so DROP and ALTER
CATALOG are not held up by a slow remote source.

#### Results

Unit tests (0 skipped). The first case runs against the real JNI bridge
and the SQLite driver, the others against a recording database:

| `AdbcClientTest` case | `close()` releasing at once, as before this PR
| This PR |
| --- | --- | --- |
| a call still on its connection when the client is closed keeps using
it | fails: `Native connection handle is closed` | passes |
| a call still reading a `getObjects` result when the client is closed
reads it to the end | fails: `the database was released under an open
connection` | passes |
| the last call to leave a closed client releases the database exactly
once, after it leaves | fails: `the database was released under an open
connection` | passes |
| a release that fails as the last call leaves does not fail that call |
fails: `the database was released under an open connection` | passes |
| an idle client releases at once | passes | passes |

Freeing only the allocator at once, and leaving the database to the last
call, fails the `getObjects` case alone: `Attempting operation on
allocator when allocator is closed`.

A stress program (not committed) closes a client 200 times while four
threads loop over `getObjects` and `getTableSchema` through
`withConnection` against the SQLite driver:

| | `AdbcClient` of master | This PR |
| --- | --- | --- |
| Runs | 5 | 5 |
| JVM crashed | 5 of 5, 1.1 to 1.3 s after the JVM started: `SIGSEGV` in
`PrivateArrowSchemaDeepCopy+0x40`, under
`ArrowArrayStreamReader.readSchema` ← `AdbcObjectsReader.readNamespaces`
← `AdbcClient.withConnection` | 0 of 5 |
| Closes completed per run | crashed before finishing | 200 |
| Metadata calls per run | crashed before reporting | 6,019 to 7,238 |
| Calls the close reached while they were using the driver | | 0 |
| Failed closes, failed deferred releases | | 0, 0 (checked with WARN
logging on, where the unit test's deliberate release failure shows up as
one warning) |

The stress runs used the macOS arm64 build of the same SQLite driver, so
the offset differs from the Linux one in CI.

#### Classes, and how they call each other

- `AdbcClient`: `withConnection` brackets the body with `enter` /
`leave`; `close` refuses new calls and releases only when idle;
`release` closes the database, then the allocator; `openDatabase` is the
one place the database is created, so a test can stand a recording
database in for the driver.
- `AdbcClientTest`: five new cases, one against the real driver and four
against a recording database. The recording connection answers
`getObjects` with a result built in memory in the client's allocator:
reading a driver's result into Arrow loads arrow-c-data's JNI shim,
which needs CXXABI_1.3.9 and does not load on the FE UT agent (see
`AdbcNativeTestSupport`, #68510).
- `AdbcObjectsReaderTest`: `response`, `catalog` and `schema` are
package-private, so `AdbcClientTest` builds that result with them.

```
STATS_FETCH: ColumnStatisticsCacheLoader.doLoad
  -> PluginDrivenExternalTable.getColumnStatistic
     -> AdbcConnectorMetadata.getTableHandle (namespaces, table names)
        -> AdbcClient.withConnection
             enter()   refused once closed; callsInFlight++
             -> AdbcConnection.getObjects -> AdbcObjectsReader.readNamespaces / readTableNames
             leave()   callsInFlight--; the last one out of a closed client -> release()

DROP CATALOG / ALTER CATALOG ... SET PROPERTIES
  -> ExternalCatalog.onClose / resetToUninitialized -> ExternalResourceReclaimer
     -> AdbcConnector.close -> AdbcClient.close()
          closed = true; release() now only if no call is in flight
          release(): AdbcDatabase.close (closes its connections and their results) -> allocator.close
```

Not changed: `AdbcConnector`, the catalog lifecycle and
`ExternalResourceReclaimer`, BE, and the regression suites;
`test_adbc_sqlite_catalog_scan` stays as it is, since what it exposes is
this bug.
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