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

feat: add the new option --memory-meta-storage #200

Merged
merged 18 commits into from
May 28, 2024
Merged

Conversation

confoc
Copy link
Contributor

@confoc confoc commented May 16, 2024

bootstrap the whole cluster without installing etcd for testing purposes through using the memory storage of metasrv in bare-metal mode.

@codecov-commenter
Copy link

codecov-commenter commented May 16, 2024

Codecov Report

Attention: Patch coverage is 0% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 40.84%. Comparing base (efa2f7d) to head (27c01d7).

Files Patch % Lines
pkg/cluster/baremetal/cluster.go 0.00% 6 Missing ⚠️
pkg/cluster/baremetal/create.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #200      +/-   ##
===========================================
- Coverage    41.01%   40.84%   -0.17%     
===========================================
  Files           16       16              
  Lines          990      994       +4     
===========================================
  Hits           406      406              
- Misses         479      483       +4     
  Partials       105      105              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@confoc confoc changed the title Pref issue #174 feat: add the new option --memory-meta-storage May 20, 2024
@MichaelScofield
Copy link
Contributor

related issue: #174

@zyy17
Copy link
Collaborator

zyy17 commented May 21, 2024

@confoc The e2e tests are failed. You should fix it.

@zyy17 zyy17 requested review from zyy17 and daviderli614 May 21, 2024 06:26
@zyy17
Copy link
Collaborator

zyy17 commented May 21, 2024

@confoc I'm confused with the PR. I can't see something about the new option of --memory-meta-storage?

pkg/components/metasrv.go Outdated Show resolved Hide resolved
cmd/gtctl/cluster_create.go Outdated Show resolved Hide resolved
cmd/gtctl/cluster_create.go Outdated Show resolved Hide resolved
@zyy17 zyy17 marked this pull request as draft May 21, 2024 06:32
@daviderli614
Copy link
Member

daviderli614 commented May 21, 2024

@confoc Change CreateTableSQL to following can fixed the e2e failling:

CREATE TABLE dist_table (
                 ts TIMESTAMP DEFAULT current_timestamp(),
                 n INT,
    		row_id INT,
                TIME INDEX (ts),
                PRIMARY KEY(n)
)
 PARTITION ON COLUMNS (n) (
           n < 5,
           n >= 5 AND n < 9,
           n >= 9
)

@daviderli614
Copy link
Member

Can add baremetal e2e test in next PR ?

@confoc confoc closed this May 21, 2024
@confoc confoc deleted the fork branch May 21, 2024 11:56
@confoc confoc restored the fork branch May 21, 2024 11:56
@daviderli614
Copy link
Member

Why close the PR ?

@confoc confoc reopened this May 22, 2024
@confoc
Copy link
Contributor Author

confoc commented May 22, 2024

I don't know how.Maybe I make some wrong operations.I don't remember, sry

@confoc
Copy link
Contributor Author

confoc commented May 22, 2024

@daviderli614 Can add baremetal e2e test in next PR ?

I have already add baremetal e2e test in the PR

@daviderli614
Copy link
Member

daviderli614 commented May 22, 2024

Seem ready review ? @confoc

@confoc confoc marked this pull request as ready for review May 23, 2024 06:31
Copy link
Collaborator

@zyy17 zyy17 left a comment

Choose a reason for hiding this comment

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

Most of the PR are good for me.

cmd/gtctl/cluster_create.go Outdated Show resolved Hide resolved
cmd/gtctl/cluster_create.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@zyy17 zyy17 left a comment

Choose a reason for hiding this comment

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

Most of the PR are good for me.

Copy link
Collaborator

@zyy17 zyy17 left a comment

Choose a reason for hiding this comment

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

lgtm

@zyy17 zyy17 merged commit ad6cf8d into GreptimeTeam:develop May 28, 2024
6 checks passed
@confoc confoc deleted the fork branch May 28, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants