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.

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

fix bug of tagless #4675

Merged
merged 2 commits into from
Sep 26, 2022
Merged

Conversation

cangfengzhs
Copy link
Contributor

@cangfengzhs cangfengzhs commented Sep 23, 2022

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

Description:

  1. when use_vertex_key=true,not save tagless key
  2. The value of Vertices in show stats needs to be deduplicated

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Tests:

  • Unit test(positive and negative cases)
  • Function test
  • Performance test
  • N/A

Affects:

  • Documentation affected (Please add the label if documentation needs to be modified.)
  • Incompatibility (If it breaks the compatibility, please describe it and add the label.)
  • If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
  • Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

ex. Fixed the bug .....

@Sophie-Xie Sophie-Xie changed the title fix bug fix bug that the value of Vertices in show stats needs to be deduplicated Sep 23, 2022
Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

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

My bad, thx~

@Sophie-Xie Sophie-Xie added the ready-for-testing PR: ready for the CI test label Sep 26, 2022
@Sophie-Xie Sophie-Xie changed the title fix bug that the value of Vertices in show stats needs to be deduplicated fix bug of tagless Sep 26, 2022
@Sophie-Xie Sophie-Xie merged commit cccc014 into vesoft-inc:master Sep 26, 2022
critical27 added a commit to critical27/nebula that referenced this pull request Oct 12, 2022
critical27 added a commit to critical27/nebula that referenced this pull request Oct 12, 2022
critical27 added a commit that referenced this pull request Oct 13, 2022
* Revert "insert vertex key when only vertex and flag is set (#4685)"

This reverts commit fff82a6.

* Revert "only write vertex key when flag is set or explictly insert vertex (#4680)"

This reverts commit a5bed33.

* Revert "fix bug (#4675)"

This reverts commit cccc014.

* Revert "Rollback vertex key (#4629)"

This reverts commit 545137a.

* wtf

* tck format
Sophie-Xie pushed a commit that referenced this pull request Oct 13, 2022
* Revert "insert vertex key when only vertex and flag is set (#4685)"

This reverts commit fff82a6.

* Revert "only write vertex key when flag is set or explictly insert vertex (#4680)"

This reverts commit a5bed33.

* Revert "fix bug (#4675)"

This reverts commit cccc014.

* Revert "Rollback vertex key (#4629)"

This reverts commit 545137a.

* wtf

* tck format
Sophie-Xie added a commit that referenced this pull request Oct 13, 2022
* Use vertex key only when use_vertex_key is on (#4716)

* Revert "insert vertex key when only vertex and flag is set (#4685)"

This reverts commit fff82a6.

* Revert "only write vertex key when flag is set or explictly insert vertex (#4680)"

This reverts commit a5bed33.

* Revert "fix bug (#4675)"

This reverts commit cccc014.

* Revert "Rollback vertex key (#4629)"

This reverts commit 545137a.

* wtf

* tck format

* fix job status when all finished (#4717)

* fix job name from zone balance to data balance

* address wenhui's comment

* fix create index if not exists behavior (#4705)

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>

Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>
Co-authored-by: Alex Xing <90179377+SuperYoko@users.noreply.github.com>
Co-authored-by: kyle.cao <kyle.cao@vesoft.com>
yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Jan 31, 2023
<!--
Thanks for your contribution!
In order to review PR more efficiently, please add information according to the template.
-->

## What type of PR is this?
- [ ] bug
- [ ] feature
- [X] enhancement

## What problem(s) does this PR solve?
#### Issue(s) number: 

#### Description:
PRD of vertex key changed again... Here is the new version of logic:
1. No matter use_vertex_key is on or off, the query result should be consistent with 3.1.
2. If use_vertex_key is on, we will use the vertex key to detect whether the vertex exists, which is the logic in 3.X version.
3. If use_vertex_key is off, we will prefix scan by vertex to detect whether the vertex exists, **which will bring performance regression**
4. We will insert the vertex only when use_vertex_key is on

> By default, use_vertex_key is off

## How do you solve it?
This PR reverts all related changes before (vesoft-inc#4685 vesoft-inc#4680 vesoft-inc#4675 vesoft-inc#4629), and apply the logic above. 

Related changes:
1. So the upgrade tool is necessary again, so bring it back.
2. The TCK cases keeps same as before vesoft-inc#4629


## Special notes for your reviewer, ex. impact of this fix, design document, etc:

> Maybe review the last commit is enough 


## Checklist:
Tests:
- [X] Unit test(positive and negative cases)
- [ ] Function test
- [ ] Performance test
- [ ] N/A

Affects:
- [X] Documentation affected (Please add the label if documentation needs to be modified.)
- [ ] Incompatibility (If it breaks the compatibility, please describe it and add the label.)
- [ ] If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
- [ ] Performance impacted: Consumes more CPU/Memory


## Release notes:

Please confirm whether to be reflected in release notes and how to describe:
> ex. Fixed the bug .....


Migrated from vesoft-inc#4716

Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants