Repository navigation
Conversation
Follow-up to the review of apache#134. The table-model editor's database, table and column pickers share the plugin's eight-session pool with panel queries. Each lookup ran again on every editor mount, could not be cancelled, and issued an extra USE. - The lookups now run SHOW TABLES FROM <db> and DESC <db>.<table>, with each part of the name quoted on its own, so they issue no USE and leave no session state behind. Panel and variable queries still USE their database. - The table-model executor checks the request context before it waits for a pooled session and again once it has one. The native client's GetSession takes no context and cannot be interrupted, so a request cancelled during the wait still waits, but it then hands the session straight back instead of running its statement. - The datasource keeps successful lookups for 30 seconds, keyed on the database and table. Opening the database list still asks the server again, and failed lookups are not kept. - The pickers pass an AbortSignal with each lookup and abort it when the picker unmounts or its database or table changes, which cancels the request. Signed-off-by: Zihan Dai <1436286758@qq.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #134 (comment): the metadata lookups behind the table-model pickers share the 8-session pool with panel queries, could not be cancelled, ran again on every editor mount, and issued an extra
USE.Statements. The table and column lookups now run
SHOW TABLES FROM "<db>"andDESC "<db>"."<table>", each part quoted on its own, so a lookup issues noUSE. Panel and variable queries stillUSEtheir database. Checked on IoTDB 2.0.11:Cancellation.
TablePickergives each lookup anAbortControllerand aborts it when the picker unmounts or its database or table changes; the signal goes throughgetResource(..., { abortSignal })and reaches the handler asr.Context(). On the backend,executeTableStatementchecks the context before taking a pooled session and again after. One limit:TableSessionPool.GetSession()in iotdb-client-go takes no context and waits up to 60 s, so a cancelled lookup still waits for a session, but it then hands the session straight back without running anything. Making the wait itself cancellable needs a context-awareGetSessionin the Go client. With all 8 sessions held, a lookup cancelled at 0.3 s returnedcontext canceledonce a session was freed at 1.5 s, and all 8 sessions could be taken again afterwards.Cache. Successful lookups are reused for 30 s, keyed on the endpoint and the names; failures are not cached. Opening the DATABASE list still asks the server again, as before. The README notes the 30 s.
Panel and variable queries go through the same executor, so they now also return the context error instead of running when their request has already been cancelled.
Tests: Go 74 -> 79 top-level tests (122 -> 135 with subtests), Jest 38 -> 46, all passing;
tsc --noEmitclean, no new ESLint findings. Removing the post-wait context check, the abort in the effect cleanup, the cache read, or the signal forwarding each makes the corresponding new test fail.