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

Implement index with timestamp #1984

Merged
merged 1 commit into from
Apr 12, 2023
Merged

Conversation

datelier
Copy link
Contributor

Description:

Add "index with timestamp" feature.

Related Issue:

Versions:

  • Go Version: 1.20.2
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 2.0.9

Checklist:

Special notes for your reviewer:

@datelier datelier requested review from kpango and hlts2 March 27, 2023 06:00
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 27, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3c2db9c
Status: ✅  Deploy successful!
Preview URL: https://13a8fe07.vald.pages.dev
Branch Preview URL: https://feature-kvs-index-with-times.vald.pages.dev

View logs

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - replace the PR body by changelog details
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase main
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

n.fmap[k] = int64(id)
}
}
for k, _:= range mt {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
File is not gofumpt-ed (gofumpt)

@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Patch coverage: 24.87% and project coverage change: -0.11 ⚠️

Comparison is base (88a417c) 29.49% compared to head (3c2db9c) 29.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1984      +/-   ##
==========================================
- Coverage   29.49%   29.38%   -0.11%     
==========================================
  Files         365      365              
  Lines       34436    34517      +81     
==========================================
- Hits        10157    10143      -14     
- Misses      23862    23955      +93     
- Partials      417      419       +2     
Impacted Files Coverage Δ
pkg/agent/core/ngt/service/ngt.go 18.94% <5.04%> (-1.54%) ⬇️
pkg/agent/core/ngt/service/kvs/ou.go 36.11% <33.33%> (ø)
pkg/agent/core/ngt/service/vqueue/queue.go 72.78% <50.00%> (ø)
pkg/agent/core/ngt/service/kvs/uo.go 56.42% <63.33%> (ø)
pkg/agent/core/ngt/service/kvs/kvs.go 82.19% <100.00%> (ø)

... and 4 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

kpango
kpango previously approved these changes Apr 3, 2023
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added team/set SET team size/XL and removed size/L labels Apr 5, 2023
@datelier datelier force-pushed the feature/kvs/index-with-timestamp branch from 181bc27 to 0d1e13c Compare April 5, 2023 01:59
@@ -32,18 +32,23 @@ type readOnlyOu struct {
}

// skipcq: GSC-G103
var expungedOu = unsafe.Pointer(new(string))
var expungedOu = unsafe.Pointer(new(ValueStructOu))
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
G103: Use of unsafe calls should be audited (gosec)

var oldVal uint32 = 10000
var (
oldVal uint32 = 10000
oldTs int64 = 20000
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
ST1003: var oldTs should be oldTS (stylecheck)

kpango
kpango previously approved these changes Apr 5, 2023
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

hlts2
hlts2 previously approved these changes Apr 6, 2023
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@datelier
Copy link
Contributor Author

/rebase

@vdaas-ci
Copy link
Collaborator

[REBASE] Rebase triggered by datelier for branch: feature/kvs/index-with-timestamp

@vdaas-ci
Copy link
Collaborator

[REBASE] Failed to rebase.

@datelier datelier dismissed stale reviews from hlts2 and kpango via 3568a14 April 10, 2023 06:21
@datelier datelier force-pushed the feature/kvs/index-with-timestamp branch from 53659cb to 3568a14 Compare April 10, 2023 06:21
hlts2
hlts2 previously approved these changes Apr 10, 2023
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

kpango
kpango previously approved these changes Apr 10, 2023
@vdaas-ci
Copy link
Collaborator

[REBASE] Failed to rebase.

kpango
kpango previously approved these changes Apr 12, 2023
@vdaas-ci
Copy link
Collaborator

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
other images

hlts2
hlts2 previously approved these changes Apr 12, 2023
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

@vdaas-ci
Copy link
Collaborator

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
other images

Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

@kpango kpango merged commit 6a1fd87 into main Apr 12, 2023
@kpango kpango deleted the feature/kvs/index-with-timestamp branch April 12, 2023 07:10
@ykadowak ykadowak mentioned this pull request Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants