Fix update sessions when leader change happens #5225
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
Aiee
added
ready-for-testing
HarrisChu
previously approved these changes
Jan 9, 2023
|
graph crash when i check the issue graph core |
critical27
previously approved these changes
Jan 10, 2023
Aiee
force-pushed
the
fix-update-session
branch
from
6bac386 to
68f75b8
Compare
critical27
reviewed
Jan 10, 2023
| LOG(ERROR) << "Update sessions failed: " << resp.status(); | ||
| return Status::Error("Update sessions failed: %s", resp.status().toString().c_str()); | ||
| } | ||
| DCHECK(resp.ok()); |
There was a problem hiding this comment.
Why do we convince that resp is always valid?
There was a problem hiding this comment.
Here we checked the response from meta service, and this guarantees handleKilledQueries() will receive a valid response.
There was a problem hiding this comment.
Implicit dependent precondition of one piece of code depends on another piece is not a good design, alghough it do works. Self-contained code is better choice.
There was a problem hiding this comment.
Implicit dependent precondition of one piece of code depends on another piece is not a good design, alghough it do works. Self-contained code is better choice.
Makes sense to me. I'll move the check into the lambda functions.
Codecov ReportBase: 76.77% // Head: 78.63% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #5225 +/- ##
==========================================
+ Coverage 76.77% 78.63% +1.86%
==========================================
Files 1109 1110 +1
Lines 82965 82984 +19
==========================================
+ Hits 63694 65258 +1564
+ Misses 19271 17726 -1545
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
yixinglu
previously approved these changes
Jan 10, 2023
|
Please hold on review, there are some issues with the log. Update: |
Aiee
force-pushed
the
fix-update-session
branch
from
a942237 to
cf3f49d
Compare
yixinglu
approved these changes
Jan 11, 2023
critical27
approved these changes
Jan 11, 2023
Sophie-Xie
pushed a commit
that referenced
this pull request
Jan 28, 2023
* Fix udpate sessions when leader change happens * Handle errors on the graph side * Address comments * Address comments
Sophie-Xie
added a commit
that referenced
this pull request
Jan 29, 2023
* optimize match node label (#5176) * revert strange return (#5183) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix stderr save error log (#5188) * fix processor_test timeout (#5180) * fix processor_test timeout * ... Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix error code (#5186) * rename the "test" space to "ngdata". (#5197) * rename the "test" space to "ngdata". * add ngdata * Revise the usages of FATAL, DFATAL, LOG, DLOG. (#5181) * Revise the usages of FATAL, DFATAL, LOG, DLOG. * fix. * revise dfatal. * Meta upgrade (#5174) * Meta upgrade remove all fulltext index when upgrade from V3 to V3_4 because of refacting of fulltext index * fix bug Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Fix pattern expression with same edge variable (#5192) * Fix pattern expression with same edge variable add tck fmt * add tck * Fix memory leak, remove toss gflag (#5204) * remove toss gflag * fix memory leak * loose wait job finish time Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add max_sessions_per_ip_per_user to default config file (#5207) * minor bug for adminTaskManager (#5195) * modify jobmanager ut (#5175) * modify jobmanager ut * add expired ut * avoid recover expired job * add ut * address review * move status Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add more match test cases on paths. (#5189) * improve memtracker, add missed check & remove unnecessary thenError&tryCatch check (#5199) * [memtracker] check code run with memoery check on all works refine code all code memory checked fix lint refine code & fix build with gcc+sanitize * fix build break * fix lint * refine code * remove debug code * fix test fail build with debug * fix test fail build with debug * restore commented test * minor * minor * fix bug (#5214) * fix bug * fix bug Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> * handle rpc error task status (#5212) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * chore: community badges refined (#5202) * chore: community badges refined * Update README-CN.md * Update README-CN.md * Update README-CN.md remove sifou and zhihu as aligned with the team * update linkedin URL Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> * Fix extend whtie space char. (#5213) * Fix extend whtie space char. * Format. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add lack tests of no role user. (#5196) Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> * remove memtracker DLOG (#5224) * Add tck cases for DDL (#5220) * more TCK tests for variable pattern match clause (#5215) * cleanup * same src/dst for variable length pattern * variable pattern in where clause * variable scope tests in path pattern * More tests * More tests Co-authored-by: jimingquan <mingquan.ji@vesoft.com> * Resumed the evaluation fo vertices in AttributeExpression (UTs included) (#5229) * add memtracker flags to conf (#5231) * add memtracker flags to conf * typo * refine * refine * add balance job type to filter when create backup (#5228) * add more job type to filter when create backup * log add job * add log before acquire snapshot lock Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Fix update sessions when leader change happens (#5225) * Fix udpate sessions when leader change happens * Handle errors on the graph side * Address comments * Address comments * fix match step range (#5216) * use smart pointer change raw pointer * fix error * fix test error * address comment Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Update response message when adding schema historically existed (#5227) * update the error code and message for checking history schemas * update tck * update comment * change to log error * fix ddl tck * increase wait time in schema.feature Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix error code (#5233) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * print memory stats default to false (#5234) * print memory stats default to false * update conf Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix bug of extract prop expr visitor (#5238) * forbid invalid prop expr used in cypher (#5242) * Fix mistake push down limit with skip. (#5241) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix delete fulltext index (#5239) * fix delete fulltext index * fix es delete error 1. remove get Rowreader if op is delete 2. delete es data when value is null Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Change the default value of session_reclaim_interval_secs to 60 seconds (#5246) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Enhance attribute-accessing expression to ensure self-consistency (#5230) * Revert "Remove all UNKNOWN_PROP as a type of null. (#4907)" (#5149) This reverts commit aa62416. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Enhance attribute-accessing expression to ensure self-consistency Fix tck Fix parser small delete Fix tck tck fmt fix ut fix ut Fix ut Fix tck Delete v.tag.prop check Fix tck Skip some tck cases related ngdata add test case Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix ft index of fixed string (#5251) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add tck test (#5253) * add allpath test * add shortest path test case * add subgraph test case * add go test case * add go test case * Add more session tests (#5256) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Revert "do not check term for leader info by default para" (#5266) This reverts commit 593bffc. * modify ft index default limit size (#5260) * modify ft index default limit size * fix test Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> * Test/yield (#5267) * Add some tests about yield. * Add more tests. Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * Add another cert to test CA don't match. (#5247) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix baton miss reset in StorageJobExecutor (#5269) * Report errors on where clauses in optional match queries. (#5273) * [test case] Check DML cases (#5264) * Check DML cases * Add chinses char tests Add more tests Add mero delete edge tests * Revert cases * fix third party version in package.sh (#5281) The dump_syms tool path should be match with third party version. * Test/user (#5139) * Add some tests about user management. * Add tests about user roles. * Format. * Fix tck fixture name. * Fix step name. * Change step name. --------- Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> * fix https (#5283) * fix memtracker bugs during stress test on graphd and storaged (#5276) * fix memtracker bugs during stress test on graphd and storaged * fix lint * fix RocksEngine memory leak of raw pointer iter * add ENABLE_MEMORY_TRACKER build option & support adaptive limit for MemoryTracker * delete debug log * refine log * refine log * fix build * refine error log * print warning if memtracker is off * fix rocksdb leak by turn off memcheck * refine synamic-self-adaptive * fix cmake check * minor * minor * minor * minor * minor * refine double equel compare --------- Co-authored-by: jimingquan <mingquan.ji@vesoft.com> Co-authored-by: jie.wang <38901892+jievince@users.noreply.github.com> Co-authored-by: Harris.Chu <1726587+HarrisChu@users.noreply.github.com> Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com> Co-authored-by: canon <87342612+caton-hpg@users.noreply.github.com> Co-authored-by: Cheng Xuntao <7731943+xtcyclist@users.noreply.github.com> Co-authored-by: hs.zhang <22708345+cangfengzhs@users.noreply.github.com> Co-authored-by: kyle.cao <kyle.cao@vesoft.com> Co-authored-by: Yichen Wang <18348405+Aiee@users.noreply.github.com> Co-authored-by: liwenhui-soul <38217397+liwenhui-soul@users.noreply.github.com> Co-authored-by: Alex Xing <90179377+SuperYoko@users.noreply.github.com> Co-authored-by: codesigner <codesigner.huang@vesoft.com> Co-authored-by: Wey Gu <weyl.gu@gmail.com> Co-authored-by: shylock <33566796+Shylock-Hg@users.noreply.github.com> Co-authored-by: pengwei.song <90180021+pengweisong@users.noreply.github.com> Co-authored-by: haowen <19355821+wenhaocs@users.noreply.github.com> Co-authored-by: George <58841610+Shinji-IkariG@users.noreply.github.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 type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Close https://github.com/vesoft-inc/nebula-ent/issues/2152, maybe relates to https://github.com/vesoft-inc/nebula-ent/issues/2176
Description:
UpdateSessions()should deal with errors like leader change.How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: