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

refactor: removing goroutine-based TTL expiry model #221

Closed
wants to merge 46 commits into from
Closed

refactor: removing goroutine-based TTL expiry model #221

wants to merge 46 commits into from

Conversation

gshilei
Copy link
Contributor

@gshilei gshilei commented Oct 18, 2023 •
edited
Loading

Hi @brandond,

According to your suggestion，I introduce the delaying work-queue to fix the "goroutine-based TTL expiry model" issue and the "watch channel of ttl events maybe abnormally closed" issue by the way. Please help to review it. if it make sense, then PR #218 can be closed in the future.

Related Issue:

In order to make the commit cleaner, please refer to new pr #235

@gshilei gshilei requested a review from a team as a code owner October 18, 2023 04:26
@gshilei gshilei changed the title refactor: import work queue to handle ttl events refactor: removing goroutine-based TTL expiry model Oct 18, 2023
brandond
brandond requested changes Oct 19, 2023
View reviewed changes
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
brandond
brandond requested changes Oct 19, 2023
View reviewed changes
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
brandond
brandond requested changes Oct 20, 2023
View reviewed changes
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

Final nit. lgtm otherwise. thanks for talking through it with me!

pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
@gshilei
Copy link
Contributor Author

gshilei commented Oct 20, 2023 •
edited
Loading

Final nit. lgtm otherwise. thanks for talking through it with me!

Thank you very much for your more professional attitude towards this review, Best wishes to you.

brandond
brandond requested changes Oct 27, 2023
View reviewed changes
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Outdated Show resolved Hide resolved
pkg/logstructured/logstructured.go Show resolved Hide resolved
pkg/logstructured/logstructured.go Show resolved Hide resolved
@gshilei gshilei requested a review from brandond October 27, 2023 17:16
@gshilei
Copy link
Contributor Author

gshilei commented Oct 30, 2023 •
edited
Loading

Hi @brandond,

Do you have any other suggestions for this PR? In addition, I found "continuous-integration/drone/pr build" was killed abnormally. How can I trigger it again?

@gshilei gshilei requested a review from brandond October 30, 2023 02:11
brandond
brandond requested changes Oct 30, 2023
View reviewed changes
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

One nit on a comment that can be removed.

Can you rebase and squash this? Once that's set I think we're good.

pkg/logstructured/logstructured.go Outdated
@@ -256,66 +268,164 @@ func (l *LogStructured) Update(ctx context.Context, key string, value []byte, re
return rev, updateEvent.KV, true, err
}

func (l *LogStructured) ttl(ctx context.Context) {
// vary naive TTL support
Copy link
Member

Choose a reason for hiding this comment

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

you can drop this comment now, I think ;)

macedogm and others added 17 commits October 31, 2023 10:31
@macedogm @gshilei
Golang and Alpine image bumps
10f33d7 
Signed-off-by: Guilherme Macedo <guilherme.macedo@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@macedogm @gshilei
Fix CI tests
0ef685f 
Signed-off-by: Guilherme Macedo <guilherme.macedo@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@matttrach @gshilei
install golangci-lint from master
6000763 
Signed-off-by: Matt Trachier <matttrach@gmail.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@matttrach @gshilei
install goimports from commit hash instead of tag
06d140a 
Signed-off-by: Matt Trachier <matttrach@gmail.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@matttrach @gshilei
remove repo info from global scope
7bebf0d 
Signed-off-by: Matt Trachier <matttrach@gmail.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@DerEnderKeks @gshilei
feat: replace lib/pq with pgx
a5553a3 
Signed-off-by: DerEnderKeks <derenderkeks@gmail.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@matttrach @gshilei
add goimports config
6a3fb67 
Signed-off-by: Matt Trachier <matttrach@gmail.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@bruth @brandond @gshilei
Add support for an embedded NATS server (k3s-io#158)
4609ef4 
* Add support for an embedded NATS server
* Put embedded server behind build flag
   Rename various identifiers to NATS rather than JetStream, which is a subsystem of NATS.
* Refactoring
* Update NATS versions and add embedded test
* Add nats build tag for tests
* Support embedded socket connection
* Update to NATS v2.9.16
* Remove unused variables
* Add backend "jetstream://" URI support
* Remove override of port
* Add examples doc
* Remove IP and port
* Fix whitespace
* Default to embedded it available
   Fix log line for embedded.
* Change to use noEmbed to disable embedding
* Fix test script and update to 2.9.16

Signed-off-by: Byron Ruth <byron@nats.io>
Co-authored-by: Brad Davidson <brad@oatmail.org>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@github-actions @gshilei
Update the version of goimports used in Dockerfile.dapper (k3s-io#182)
ac2c6eb 
* chore: Updated the content of the file "/tmp/updatecli/github/k3s-io/kine/Dockerfile.dapper"

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dependabot @gshilei
Bump github.com/rancher/wrangler from 0.8.3 to 0.8.5
42e563c 
Bumps [github.com/rancher/wrangler](https://github.com/rancher/wrangler) from 0.8.3 to 0.8.5.
- [Release notes](https://github.com/rancher/wrangler/releases)
- [Commits](rancher/wrangler@v0.8.3...v0.8.5)

---
updated-dependencies:
- dependency-name: github.com/rancher/wrangler
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@bruth @gshilei
Revert legacy support for jetstream:// scheme
bedc194 
Existing deployments using this scheme will effectively ignore the
embedded server options.

Signed-off-by: Byron Ruth <byron@nats.io>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@matttrach @gshilei
Add dependabot label and reviewer
bfabc68 
Signed-off-by: Matt Trachier <matttrach@gmail.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@vitorsavian @gshilei
Adding makefile
a3cc4ed 
Signed-off-by: Vitor Savian <vitor.savian@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dependabot @gshilei
Bump alpine from 3.17 to 3.18 in /package
0889371 
Bumps alpine from 3.17 to 3.18.

---
updated-dependencies:
- dependency-name: alpine
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@bbigras @gshilei
examples/nats.md: fix typo
f0c5678 
Signed-off-by: Bruno Bigras <bigras.bruno@gmail.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@bruth @gshilei
Update NATS to v2.9.18 (k3s-io#192)
0e9e5a4 
* Update NATS to v2.9.18
* Update test-run-nats to 2.9.18

Signed-off-by: Byron Ruth <byron@nats.io>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@github-actions @gshilei
Update the version of goimports used in Dockerfile.dapper (k3s-io#187)
853f002 
* chore: Updated to content "" in file "Dockerfile.dapper"

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
brandond and others added 27 commits October 31, 2023 10:31
@brandond @gshilei
Bump sql and etcd module versions
176b49b 
Updates mysql=v1.2.7, pgx=v5.4.2, go-sqlite3=v1.14.17/sqlite=3.42.0, etcd=v3.5.9

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@github-actions @gshilei
Update the version of goimports used in Dockerfile.dapper (k3s-io#200)
77f3a7b 
* chore: Updated to content "" in file "Dockerfile.dapper"
* chore: Updated to content "" in file "Dockerfile.dapper"

Made with ❤️️ by updatecli

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@brandond @gshilei
fix github-release step for arm
3f90ec9 
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dereknola @gshilei
Bump nats
183204e 
Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dereknola @gshilei
Only bump nats.go
f87db20 
Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@gshilei
Fix SQLLog.compactor
2bb7e58 
Because it will cause memory leak if we do not stop ticker when the function has completed.

Signed-off-by: hang.jiang <hang.jiang@daocloud.io>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@moio @gshilei
SQLite: specify a busy timeout
2997094 
Under stress, concurrent operations (notably while garbage collection is in progress) can result in failed calls with error SQLITE_BUSY (database is locked).

The SQLITE_BUSY result code is an expected outcome when the database file could not be written (or in some cases read) because of concurrent activity: https://www.sqlite.org/rescode.html

SQLite ships a busy timeout retry mechanism as a PRAGMA (https://www.sqlite.org/pragma.html#pragma_busy_timeout), that can be set via go-sqlite3's connection string.

That makes go-sqlite3 retry operations that fail with SQLITE_BUSY transparently to users.

Fixes k3s-io/k3s#8234

Signed-off-by: Silvio Moioli <silvio@moioli.net>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@caroline-suse-rancher @gshilei
Create CODEOWNERS
155eb9d 
Add CODEOWNERS, MAINTAINERS and enable DCO.

Signed-off-by: Caroline Davis <caroline.davis@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dependabot @gshilei
Bump golang from 1.20-alpine3.17 to 1.21-alpine3.17
b98d87f 
Bumps golang from 1.20-alpine3.17 to 1.21-alpine3.17.

---
updated-dependencies:
- dependency-name: golang
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@github-actions @gshilei
Update the version of goimports used in Dockerfile.dapper (k3s-io#202)
1612259 
* chore: Updated to content "" in file "Dockerfile.dapper"

Made with ❤️️ by updatecli

Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@testwill @gshilei
chore: remove refs to deprecated io/ioutil (k3s-io#223)
97fd52a 
Signed-off-by: guoguangwu <guoguangwu@magic-shield.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dependabot @gshilei
Bump actions/checkout from 3 to 4 (k3s-io#212)
b828efb 
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dereknola @gshilei
Remove dapper from testing, update DBs tested (k3s-io#232)
b5c6d07 
* Remove dapper from testing
* Bump default k3s used for testing
* Update mysql and postgres versions to test

Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@brandond @gshilei
Fix issue with gap fill records clobbering inserts under load on pgsql
2a9c525 
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@brandond @gshilei
Remove http/tls mux from etcd client listener
62b033e 
Upstream etcd has seen issues with grpc muxed with http and recommends against it. Ref: etcd-io/etcd#15402

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@brandond @gshilei
Don't log error if watch cancel can't be sent because the connection …
e210f39 
…was closed

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@brandond @gshilei
Add check for mod revision=0 in update txn
d0d35c9 
Also, return proper GPRC errors to improve client handling.

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@brandond @gshilei
Properly return ErrCompacted/ErrFutureRev for get/list
4a8a716 
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dependabot @gshilei
Bump google.golang.org/grpc from 1.53.0 to 1.56.3
3844373 
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.53.0 to 1.56.3.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.53.0...v1.56.3)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dependabot @gshilei
Bump golang.org/x/net from 0.10.0 to 0.17.0
a8034d6 
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.10.0 to 0.17.0.
- [Commits](golang/net@v0.10.0...v0.17.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dereknola @gshilei
Actually run postgres tests (k3s-io#234)
a241847 
Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@gshilei
rebase master
7e9240e 
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@gshilei
refactor: enhance handling ttl events
0937186 
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@gshilei
refactor: change sync.map to map with RWMutex
ecb6679 
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@gshilei
refactor: fix comments
a4c701b 
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dereknola @gshilei
Remove dapper from testing, update DBs tested (k3s-io#232)
8e66806 
* Remove dapper from testing
* Bump default k3s used for testing
* Update mysql and postgres versions to test

Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@dereknola @gshilei
Actually run postgres tests (k3s-io#234)
88c55dc 
Signed-off-by: Derek Nola <derek.nola@suse.com>
Signed-off-by: zhiyuan <guoshilei.gsl@antgroup.com>
@gshilei gshilei mentioned this pull request Oct 31, 2023
Merged
@gshilei gshilei closed this Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandond brandond brandond requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.