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

Close toss, and some other minor enhancement #5119

Merged
merged 10 commits into from
Dec 28, 2022
Merged

Close toss, and some other minor enhancement #5119

merged 10 commits into from
Dec 28, 2022

Conversation

critical27
Copy link
Contributor

@critical27 critical27 commented Dec 27, 2022 •
edited
Loading

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

Description:

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 .....

@critical27 critical27 requested a review from a team as a code owner December 27, 2022 03:42
@critical27 critical27 added ready-for-testing PR: ready for the CI test ready for review labels Dec 27, 2022
@critical27 critical27 force-pushed the toss branch 2 times, most recently from 61afc40 to a5ce238 Compare December 27, 2022 03:50
SuperYoko
SuperYoko previously approved these changes Dec 27, 2022
View reviewed changes
codesigner
codesigner reviewed Dec 27, 2022
View reviewed changes
Copy link
Contributor

@codesigner codesigner left a comment

Choose a reason for hiding this comment

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

LGTM,

src/tools/storage-perf/CMakeLists.txt
)
#nebula_add_executable(
# NAME
# storage_perf
Copy link
Contributor

Choose a reason for hiding this comment

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

why comment out this tools (simple_kv_verify, storage_perf, storage_integrity), is it related that toss is turned off?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not related to toss, these tools are out of date now, which has not been maintained for a long time.

@critical27
Copy link
Contributor Author

TCK failed, hold on a sec.

@critical27
Copy link
Contributor Author

Ready now, verified in local.

liwenhui-soul
liwenhui-soul previously approved these changes Dec 27, 2022
View reviewed changes
@Sophie-Xie
Copy link
Contributor

How about the query_concurrently flag? 😂

@critical27
Copy link
Contributor Author

How about the query_concurrently flag? 😂

Added, with default value false

@wenhaocs
Copy link
Contributor

Awesome!

wenhaocs
wenhaocs previously approved these changes Dec 27, 2022
View reviewed changes
@wenhaocs
Copy link
Contributor

How about the query_concurrently flag? 😂

Added, with default value false

I see argument about this in the ticket. Please confirm.

@critical27 critical27 dismissed stale reviews from wenhaocs and liwenhui-soul via b69bf50 December 28, 2022 03:33
@critical27
Copy link
Contributor Author

How about the query_concurrently flag? 😂

Added, with default value false

I see argument about this in the ticket. Please confirm.

After discuss, will make it true by default

@Sophie-Xie Sophie-Xie merged commit 6cb56c3 into vesoft-inc:master Dec 28, 2022
@Sophie-Xie Sophie-Xie added the doc affected PR: improvements or additions to documentation label Dec 30, 2022
@wey-gu wey-gu mentioned this pull request Dec 31, 2022
Closed
@foesa-yang foesa-yang self-assigned this Jan 12, 2023
@mo-avatar mo-avatar mentioned this pull request Jan 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liwenhui-soul liwenhui-soul liwenhui-soul approved these changes

@wenhaocs wenhaocs wenhaocs approved these changes

@codesigner codesigner codesigner approved these changes

@SuperYoko SuperYoko SuperYoko left review comments

@cangfengzhs cangfengzhs Awaiting requested review from cangfengzhs

@pengweisong pengweisong Awaiting requested review from pengweisong

@dutor dutor Awaiting requested review from dutor dutor is a code owner

Assignees

@foesa-yang foesa-yang

Labels
doc affected PR: improvements or additions to documentation ready for review ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@critical27 @Sophie-Xie @wenhaocs @codesigner @liwenhui-soul @SuperYoko @foesa-yang