Add limit clause to pinot queries #5337
Merged
Conversation
This file contains 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
neil-xie
pushed a commit
that referenced
this pull request
Aug 3, 2023
* add limit clause to queries * cleanup: run make copyright things
neil-xie
pushed a commit
that referenced
this pull request
Aug 15, 2023
* add limit clause to queries * cleanup: run make copyright things
neil-xie
pushed a commit
that referenced
this pull request
Sep 28, 2023
* add limit clause to queries * cleanup: run make copyright things
bowenxia
added a commit
that referenced
this pull request
Nov 3, 2023
* Add pinot dual visibility manager and new advance visibility option * update visibility store and implemented unit test * update: go mod tidy * update go.sum * Fix the start options for pinot * update unit test * run make cmds to update files * update one unit test case that had errors * fix unit test * Fix pinot config file and add ES config to make it run with workers and frontend * add ES into docker compose file * Fix kafka producer message * revert kafka change, update producer message struct to match with schema * cleanup * change tableName and places used it accordingly, to allow pinot to recognize the name * add a pinotClientInterface, refactor unit test * NewPinotConnectionClient should return a genetic value * fix a failed unit test & add mock genericClient component * rename pinotConnectionClient tobe pinotClient * change PinotClient to be public * Update visibility manager to use the new pinot generic client * Fix the log format * update kafka config to separate kafka topics for pinot and ES, add pinot visibility triple manager * Fix typo in pinot table config * change json tags to start with upper case to match pinot schema * Fix run ID json tag and remove unused kafka key from schema * fix a naming issue that cause pinot can't receive CloseTime * change json tag so that closeStatus won't be ignored when it is 0 * add attr into pinot * update decoded attr * Update reading from pinot dynamic config * add single quote to query * correct order Query formations * Add log for debug purpose * fix can't unmarshal request.Attr error * fix nil pointer in isRecordValid * clean up * Remove unnecessary debug info and unused message fields * solve can't unmarshal attr issue * update unit test to pass * Get pinot table from config * Update table name in config * use table name from config * clean up * update unit test * change couple types in pinot message * update pinot visibility triple manager to write to pinot and ES (#5229) * Cdnc 4574 (#5230) * update pinot config to enable text_index so that we could use Text_match function in query * implement customized attribute search * delete an unused function * change name to avoid build fail * use reg exp to split with case insensitive 'and' * clean up comment * clean up * Add pagination and flatten customized search attributes (#5234) * implement pagination * add a test case for nextPageToken * add more test cases for pagination * change elastic search token into pinot token * clean up * fix count didn'w work issue * try to implement flatten schema, add attributes into schema * add attributes into schema * add some code to convert time to unix time in customized search attributes * refactor previously added code to a function * customized search attributes and unit tests * Remove attr column and minor clean up * edge case for ORDER BY in customized search attribute * Fix typo in name * add one more condition for parseLastElement * split one element of customized search attribute by operator instead of space; add a filter for customized attributes prefix * update unit test * update unit test --------- Co-authored-by: Neil Xie <neil.xie@uber.com> * Adds Dynamic-config type (#5261) - Refactors the config-store library to include a new dimension called 'configType' which allows for multiple stores. - Refactors the config-store slightly to reflect the fact that it is indeed actually a daemon These changes are tested and part of a larger set of changes of the 'zonal-isolation' feature * clean up: delete one unused function, and one line refactor * update config file for deleting Attr * fix a nil pointer after removing Attr * update a test case to cover multiple order by clause case * Fix fmt * Add pinot integration test (#5316) * Add integration test for pinot and fix bugs * Fix frontend to read from pinot when set up pinot test cluster * Add a new map for Attr in response --------- Co-authored-by: Bowen Xiao <xbowen@uber.com> * Cdnc 4589 (#5318) * Add pinot dual visibility manager and new advance visibility option * update visibility store and implemented unit test * update: go mod tidy * update go.sum * Fix the start options for pinot * update unit test * run make cmds to update files * update one unit test case that had errors * fix unit test * Fix pinot config file and add ES config to make it run with workers and frontend * add ES into docker compose file * Fix kafka producer message * revert kafka change, update producer message struct to match with schema * cleanup * change tableName and places used it accordingly, to allow pinot to recognize the name * add a pinotClientInterface, refactor unit test * NewPinotConnectionClient should return a genetic value * fix a failed unit test & add mock genericClient component * rename pinotConnectionClient tobe pinotClient * change PinotClient to be public * Update visibility manager to use the new pinot generic client * Fix the log format * update kafka config to separate kafka topics for pinot and ES, add pinot visibility triple manager * Fix typo in pinot table config * change json tags to start with upper case to match pinot schema * Fix run ID json tag and remove unused kafka key from schema * fix a naming issue that cause pinot can't receive CloseTime * change json tag so that closeStatus won't be ignored when it is 0 * add attr into pinot * update decoded attr * Update reading from pinot dynamic config * add single quote to query * correct order Query formations * Add log for debug purpose * fix can't unmarshal request.Attr error * fix nil pointer in isRecordValid * clean up * Remove unnecessary debug info and unused message fields * solve can't unmarshal attr issue * update unit test to pass * Get pinot table from config * Update table name in config * use table name from config * clean up * update unit test * change couple types in pinot message * update pinot visibility triple manager to write to pinot and ES (#5229) * Cdnc 4574 (#5230) * update pinot config to enable text_index so that we could use Text_match function in query * implement customized attribute search * delete an unused function * change name to avoid build fail * use reg exp to split with case insensitive 'and' * clean up comment * clean up * Add pagination and flatten customized search attributes (#5234) * implement pagination * add a test case for nextPageToken * add more test cases for pagination * change elastic search token into pinot token * clean up * fix count didn'w work issue * try to implement flatten schema, add attributes into schema * add attributes into schema * add some code to convert time to unix time in customized search attributes * refactor previously added code to a function * customized search attributes and unit tests * Remove attr column and minor clean up * edge case for ORDER BY in customized search attribute * Fix typo in name * add one more condition for parseLastElement * split one element of customized search attribute by operator instead of space; add a filter for customized attributes prefix * update unit test * update unit test --------- Co-authored-by: Neil Xie <neil.xie@uber.com> * Adds Dynamic-config type (#5261) - Refactors the config-store library to include a new dimension called 'configType' which allows for multiple stores. - Refactors the config-store slightly to reflect the fact that it is indeed actually a daemon These changes are tested and part of a larger set of changes of the 'zonal-isolation' feature * clean up: delete one unused function, and one line refactor * update config file for deleting Attr * fix a nil pointer after removing Attr * update a test case to cover multiple order by clause case * Fix fmt * Add integration test for pinot * Fix frontend to read from pinot when set up pinot test cluster * Update history to load pinot config correctly * add more test cases/most of them are failing * fix WorkflowExecutionCloseStatus unmarshal panic * add a new map for Attr in response * pass upsert tests * scanWorkflow pass * pass listOpenWorkflow * pass pagination tests * all test passed * BinaryChecksums support arrays * make go-generate && make fmt && make lint && make copyright * Minor tweaks * Clean up * handle CloseTime = missing case * only deal with system keys * delete single quote for search val if it is not string * cleanup * refactor utility functions to a new file for both OSS and Mono repo to use * run make go-generate && make fmt && make lint && make copyright --------- Co-authored-by: Neil Xie <neil.xie@uber.com> Co-authored-by: neil-xie <104041627+neil-xie@users.noreply.github.com> Co-authored-by: David Porter <david.porter@uber.com> * Update Pinot query to order by closetime when query closed wf, order by runID when query open wf * refactor pinotClient to pass in pinotConfig * Revert "refactor pinotClient to pass in pinotConfig" This reverts commit 4f403b2. * refactor pinotClient to pass in pinotConfig * PinotQueryValidator (#5333) * add pinotQueryValidator, and change default order by to be StartTime * clean up: delete comments * clean up: run make copy right things * refactor PinotQueryValidator * refactor * clean up: run copyright things * Add limit clause to pinot queries (#5337) * add limit clause to queries * cleanup: run make copyright things * Update all queries to order by startTime * Adding a PInot/ES response comparator (#5353) * add response comparator *when comparison fails, it will return es result and log the error * if can't start both mgr, use only one mgr * Refactor code to determine read mode and which visibility manager to use when read mode is both * Return valid response when one of the source is broken --------- Co-authored-by: Neil Xie <neil.xie@uber.com> * Fix rebase and lint * Fix integration test and minor clean up * more clean up * Add more comments and more clean up * Update to use constants for visibility store name instead of strings * Enable json index (#5390) * enable json index, change the quries for exact/partial match, update the unit tests * make it pass integration test: found that order by is not supported in json index column * update partial match query. It didn't work in the mono repo * Implemented deletion method for Pinot visibility store (#5404) * Rebase * Uncomment code that caused error by idl changes * More clean up * turn off comparator * Add pinot metrics client and update pinot visibility manager to use it (#5411) * Add pinot metrics client and update pinot visibility manager to use it * Update read and write mode to prepare for migration * Add SecondsSinceEpoch field and update Pinot schema (#5418) * Address comments part 1 * Address comments and fix Pinot integration test * remove temporarily to rename folder * Add back with new folder name * Fix * Minor fix for stopwatch * Add more comments * add range query and unit test * support <, >, >=, <= for custom attributes * remove dead code * add unit tests * add comment for range query function --------- Co-authored-by: Neil Xie <neil.xie@uber.com> Co-authored-by: neil-xie <104041627+neil-xie@users.noreply.github.com> Co-authored-by: David Porter <david.porter@uber.com> Co-authored-by: Shijie Sheng <shengs@uber.com>
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.
What changed?
Added limit clause into all queries expect for getCloseWorkflow & countWorkflow APIs
Why?
Pinot production table was crashed because of lack of limit clause
How did you test it?
Unit test,
Integration test
Potential risks
Release notes
Documentation Changes