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

IGNITE-20454 Sql. Add a query prefetch callback that is notified when data prefetching is complete. #2674

Merged
merged 9 commits into from Oct 19, 2023
Merged

IGNITE-20454 Sql. Add a query prefetch callback that is notified when data prefetching is complete. #2674

merged 9 commits into from Oct 19, 2023

Conversation

xtern
Copy link
Contributor

@xtern xtern commented Oct 10, 2023 •
edited

@xtern xtern changed the title IGNITE-20454 IGNITE-20454 Sql. Extend SQL cursor with ability to check if first page is ready Oct 11, 2023
@lowka
Copy link
Contributor

lowka commented Oct 11, 2023 •
edited

@xtern Maybe we can add a test case to ExecutionServiceImplTest that exercises that this listener is called?

@xtern xtern force-pushed the ignite-20454 branch 8 times, most recently from b0e83af to 6e57d0a Compare October 12, 2023 09:41
@xtern xtern requested a review from lowka October 12, 2023 11:56
@xtern xtern requested a review from korlov42 October 13, 2023 13:05
korlov42
korlov42 requested changes Oct 16, 2023
View reviewed changes
Copy link
Contributor

@korlov42 korlov42 left a comment

Choose a reason for hiding this comment

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

The goal of the JIRA is to eliminate dependency on the user's input while processing the script. AsyncSqlCursorImpl#requestNextAsync is a user input for sure. Besides, the description of the ticket points out to a particular place that triggers prefetch (AsyncRootNode#prefetch). End of the prefetch is the moment when waiting reaches 0 for the first time. So, we need to trigger listeners when waiting reaches 0 after AsyncRootNode#prefetch was invoked.

BTW, I think, the name listener is more suitable for recurrent events, while for single shot notification 'callback' fits better

@xtern xtern force-pushed the ignite-20454 branch 2 times, most recently from a5abba4 to 30f365f Compare October 16, 2023 16:52
@xtern xtern marked this pull request as draft October 16, 2023 19:51
@xtern xtern marked this pull request as ready for review October 17, 2023 05:38
@xtern xtern force-pushed the ignite-20454 branch 3 times, most recently from 2f6eec3 to 099951e Compare October 17, 2023 08:17
@xtern xtern changed the title IGNITE-20454 Sql. Extend SQL cursor with ability to check if first page is ready IGNITE-20454 Sql. Add a query prefetch callback that is notified when data prefetching is complete. Oct 17, 2023
@xtern
Copy link
Contributor Author

xtern commented Oct 17, 2023

The goal of the JIRA is to eliminate dependency on the user's input while processing the script. AsyncSqlCursorImpl#requestNextAsync is a user input for sure. Besides, the description of the ticket points out to a particular place that triggers prefetch (AsyncRootNode#prefetch). End of the prefetch is the moment when waiting reaches 0 for the first time. So, we need to trigger listeners when waiting reaches 0 after AsyncRootNode#prefetch was invoked.

BTW, I think, the name listener is more suitable for recurrent events, while for single shot notification 'callback' fits better

@korlov42

Yes 🤦‍♀️, thanks for the detailed explanation.

I've reworked the patch, please have a look.

@xtern xtern requested a review from korlov42 October 17, 2023 13:17
@xtern xtern merged commit 8779209 into apache:main Oct 19, 2023
1 check passed
@xtern xtern deleted the ignite-20454 branch October 19, 2023 10:54
Lozmanov pushed a commit to Lozmanov/ignite-3 that referenced this pull request Nov 6, 2023
@github-actions @alievmirza
@ascherbakoff @tkalkirill @korlov42 @rpuch @lowka @Cyrill @Flaugh24 @vldpyatkov @isapego @sanpwc @ygerzhedovich @PakhomovAlexander @xtern @zstan @denis-chudov @Pochatkin @valepakh @sashapolo @ptupitsyn @AMashenkov @IgGusev
Merge Apache Ignite to GG9 (#41)
32b2386 
* IGNITE-19425 Use data nodes from DistributionZoneManager instead of BaselineManager#nodes and remove BaselineManager (apache#2605)

* IGNITE-20127 Implement delayed replication acks - Fixes apache#2520.

Signed-off-by: Alexey Scherbakov <alexey.scherbakoff@gmail.com>

* IGNITE-20330 Create an abstraction for building indexes (apache#2631)

* IGNITE-20513 Use NativeType as the type of the system view column (apache#2668)

* IGNITE-20590 Remove TxManager.beginImplicit() (apache#2669)

* IGNITE-20588 Use consistent schema when converting data in KV/record views (apache#2667)

* IGNITE-20342 Rollback transaction for SQL execution issues (apache#2611)

* IGNITE-20445 Clean up write intents for RW transaction on replication… (apache#2657)

* IGNITE-20607 Move NativeType to core (apache#2670)

* IGNITE-20484 NPE when some operation occurs when the primary replica … (apache#2672)

* IGNITE-20442 Sql. Extend grammar with transaction related statements. (apache#2663)

* IGNITE-20620 Add index availability command to catalog (apache#2680)

* IGNITE-20521 Split Security API from ignite-security module (apache#2662)

* IGNITE-20621 Add a replication group index build completion listener to IndexBuilder (apache#2682)

* IGNITE-19226 Fetch table schema by timestamp (apache#2648)

* IGNITE-19325 Unmute test MultiActorPlacementDriverTest#prolongAfterActiveActorChanger (apache#2677)

* IGNITE-20625 Fix ODBC metadata reading error (apache#2687)

* IGNITE-20457 Verify commitTimestamp against enlisted partitions expiration timestamps (apache#2658)

* IGNITE-20635 Cleanup code wrt IGNITE-18733 mentions (apache#2686)

* IGNITE-20317 Return metastorage invokes for zones changes in handlers, immediately recalculate data nodes when scale up/down is immediate. (apache#2685)

* IGNITE-19276 Implement a mechanism to build indices distributively (apache#2676)

* IGNITE-20567 Move the 'enabled' flag from the authentication configuration to security (apache#2665)

* IGNITE-20578 Implement scan over system view (apache#2678)

* IGNITE-20387: Remap most exceptions to SqlExceptions for SQL API (apache#2613)

* IGNITE-19217 Implement foreign keys query for ODBC  (apache#2692)

* IGNITE-20512 Remove port range from HTTP server (apache#2673)

* Remove port range from HTTP server configuration
* Update junit5 version to avoid jar hell.

* IGNITE-20657 At the checkpoint ArrayIndexOutOfBoundsException (apache#2696)

* IGNITE-20444 Sql. Add restrictions for execution tx related statements with single statement mode (apache#2683)

* IGNITE-20116 Linearize storage updates with safeTime adjustment rules (apache#2689)

* IGNITE-20636 Add to the MakeIndexAvailableCommand the ability to use only the indexId (apache#2691)

* IGNITE-19219  Implement primary keys query in ODBC (apache#2699)

* IGNITE-20366 testBatchReadPutConcurrently failed (apache#2694)

* IGNITE-20435 Preserve key order in batch opperations (deleteAll, deleteAllExact, insertAll) (apache#2664)

* IGNITE-20385 Sql. Fixed handling of NodeLeftException (apache#2622)

* IGNITE-20672 Broke compilation (apache#2705)

* IGNITE-20530 Start building indexes for write-only indexes (apache#2704)

* IGNITE-20659 Placement driver do not create a lease (apache#2700)

* IGNITE-20668 Increase wait after a DDL to account for idle safe-time propagation period (apache#2703)

* IGNITE-20522 Create the default user 'Ignite' (apache#2684)

* Ignite/ignite default user is added to the configuration if the user did not specify one
* security.authentication.enabled -> security.enabled
* We didn't have the ability to set dynamic defaults in the current implementation of configuration before this patch. Moreover, we don't have the ability to set a default value for NamedConfigValue at all. Now you can add the code to the configuration module if you want to add some defaults based on user-provided values or other external conditions.

* IGNITE-20616 ensureReplicaIsPrimary should use getPrimaryReplica instead of awaitPrimaryReplica (apache#2701)

* IGNITE-20671 Sql. Fixed ItSqlApiTest#ddl test (apache#2708)

* IGNITE-20677 Sql. Improve error reporting in assertThrowsSqlException (apache#2707)

* IGNITE-20600 Sql. Fix a message of an error, which occurs while updating primary key column (apache#2695)

* IGNITE-20478 Sql. Get rid of UNSPECIFIED_VALUE_PLACEHOLDER (apache#2671)

* IGNITE-20430 Got rid of unused set and fixed replica waiters removal (apache#2604)

* IGNITE-20454 Sql. Added a callback that is notified when data prefetching is complete (apache#2674)

* IGNITE-20002 Implement durable unlock on primary partition re-election (apache#2697)

* IGNITE-20630 Select only available nodes for deployment unit download (apache#2713)

* IGNITE-20693 Fixed NPE in placement driver actor on deactivation (apache#2718)

* IGNITE-20695 Cleanup resource (apache#2723)

* IGNITE-20545 Improve logging in AbstractRpcTest (apache#2717)

* IGNITE-20395 Clean up write intents for RW transaction on primary (apache#2679)

* IGNITE-20699 Decrease idle safe time propagation period in tests (apache#2730)

* IGNTIE-20629 Exclude ODBC build from assemble pipeline by default (apache#2706)

* IGNITE-20706 Rename CatalogSchemaManager to SchemaManager (apache#2733)

* IGNITE-20702 Fix NPE in ReplicaManager.onReplicaMessageReceived (apache#2731)

* IGNITE-20704 Add methods to fsync files and directories (apache#2735)

* IGNITE-20703 LeaseUpdaterTest causes a NullPointerException (apache#2732)

* IGNITE-20599 Implement 'NOT_TOMBSTONE' operation in the meta storage dsl. (apache#2722)

* IGNITE-20042 Check table existence before executing each operation in an RW transaction (apache#2721)

* IGNITE-20576 Fix testGetReturningTupleWithUnknownSchemaRequestsNewSchema (apache#2738)

* IGNITE-20359 Expose storage profiles as a node attribute (apache#2711)

* IGNITE-20439 Sql. Support multiple schemas in CatalogSqlSchemaManager (apache#2719)

* IGNITE-20311 Sql. Fix behaviour of ROUND function (apache#2690)

* IGNITE-20684 Use proper sync method (apache#2715)

* IGNITE-20670 Extract deployment code integration test to related module (apache#2714)

* IGNITE-20304 Sql. Documentation for system views module (apache#2726)

* IGNITE-20644 Java thin: Fix error detection in doSchemaOutInOpAsync (apache#2739)

Fix well-known error detection in `ClientTable` when `sendServerExceptionStackTraceToClient` is enabled: `unwrapRootCause` did not work, the important exception can be on any level.

* IGNITE-20498 Fix potential catalog version order violations (apache#2734)

* IGNITE-20560 Remove the field out of sync with the map (apache#2660)

* IGNITE-20099 Update okhttp version (apache#2746)

* IGNITE-20720 Move ClusterPerTestIntegrationTest and ClusterPerClassIntegrationTest to Test Fixtures (apache#2744)

* IGNITE-20618 Sql. Degradation of SELECT operations performance over time (apache#2728)

* IGNITE-20726: Fix incorrect link that mentions a closed ticket. (apache#2745)

* IGNITE-20727 Pass schema version in each read/write ReplicaRequest (apache#2747)

* IGNITE-20561 Change condition for DistributionZonesUtil#triggerKeyConditionForZonesChanges to solve inconsistency issues (apache#2743)

* IGNITE-20734 Fixed a link to cli doc (apache#2750)

* IGNITE-20739 fix compilation after merge of IGNITE-20561 (apache#2753)

* IGNITE-20624 Fix race between getting logical topology and mapping fragments (apache#2710)

---------

Signed-off-by: Alexey Scherbakov <alexey.scherbakoff@gmail.com>
Co-authored-by: Mirza Aliev <alievmirza@gmail.com>
Co-authored-by: Alexey Scherbakov <alexey.scherbakoff@gmail.com>
Co-authored-by: Kirill Tkalenko <tkalkirill@yandex.ru>
Co-authored-by: korlov42 <korlov@gridgain.com>
Co-authored-by: Roman Puchkovskiy <roman.puchkovskiy@gmail.com>
Co-authored-by: Max Zhuravkov <shhwwa@gmail.com>
Co-authored-by: Cyrill <cyrill.sizov@gmail.com>
Co-authored-by: Ivan Gagarkin <gagarkin.iiu@gmail.com>
Co-authored-by: Vladislav Pyatkov <vldpyatkov@gmail.com>
Co-authored-by: Igor Sapego <isapego@apache.org>
Co-authored-by: Alexander Lapin <lapin1702@gmail.com>
Co-authored-by: ygerzhedovich <41903880+ygerzhedovich@users.noreply.github.com>
Co-authored-by: Aleksandr Pakhomov <apkhmv@gmail.com>
Co-authored-by: Pavel Pereslegin <xxtern@gmail.com>
Co-authored-by: Evgeniy Stanilovskiy <stanilovsky@gmail.com>
Co-authored-by: Denis Chudov <moonglloom@gmail.com>
Co-authored-by: Mikhail <Pochatkin@users.noreply.github.com>
Co-authored-by: Vadim Pakhnushev <8614891+valepakh@users.noreply.github.com>
Co-authored-by: Alexander Polovtcev <alex.polovtcev@gmail.com>
Co-authored-by: Pavel Tupitsyn <ptupitsyn@apache.org>
Co-authored-by: Andrew V. Mashenkov <AMashenkov@users.noreply.github.com>
Co-authored-by: IgGusev <igusev@gridgain.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lowka lowka lowka approved these changes

@korlov42 korlov42 korlov42 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@xtern @lowka @korlov42