-
Notifications
You must be signed in to change notification settings - Fork 89
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: optimize cache performance #519
Conversation
# Conflicts: # http/middleware/throttle.go
WalkthroughThe recent updates primarily enhance the cache system by switching from Changes
Poem
Tip AI model upgrade
|
@coderabbitai review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
mocks/cache/Cache.go (2)
Line range hint
77-115
: Consider replacing panic with graceful error handling.The panic statement for missing return values in the
Decrement
method is aggressive. Consider returning an error instead.- if len(ret) == 0 { - panic("no return value specified for Decrement") - } + if len(ret) == 0 { + return 0, errors.New("no return value specified for Decrement") + }
Line range hint
635-673
: Consider replacing panic with graceful error handling.The panic statement for missing return values in the
Increment
method is aggressive. Consider returning an error instead.- if len(ret) == 0 { - panic("no return value specified for Increment") - } + if len(ret) == 0 { + return 0, errors.New("no return value specified for Increment") + }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (8)
- cache/driver_test.go (2 hunks)
- cache/memory.go (6 hunks)
- cache/memory_test.go (3 hunks)
- contracts/cache/cache.go (2 hunks)
- go.mod (4 hunks)
- grpc/application.go (2 hunks)
- mocks/cache/Cache.go (6 hunks)
- mocks/cache/Driver.go (6 hunks)
Additional comments not posted (64)
contracts/cache/cache.go (2)
17-17
: Update documentation forDecrement
method.The documentation comment for the
Decrement
method should be updated to reflect the newint64
type.- // Decrement decrements the value of an item in the cache. + // Decrement decrements the value of an item in the cache by an `int64` value.
37-37
: Update documentation forIncrement
method.The documentation comment for the
Increment
method should be updated to reflect the newint64
type.- // Increment increments the value of an item in the cache. + // Increment increments the value of an item in the cache by an `int64` value.grpc/application.go (1)
92-92
: Ensure compatibility with gRPC interceptors.The change from
grpcmiddleware.ChainUnaryServer
togrpc.ChainUnaryInterceptor
should be verified for compatibility with existing interceptors.Ensure that all existing unary server interceptors are compatible with
grpc.ChainUnaryInterceptor
.cache/driver_test.go (2)
82-82
: Update test cases forDecrement
method.Ensure that the test cases for the
Decrement
method are updated to handleint64
values.Ensure that all test cases for
Decrement
method are updated to handleint64
values.
131-131
: Update test cases forIncrement
method.Ensure that the test cases for the
Increment
method are updated to handleint64
values.Ensure that all test cases for
Increment
method are updated to handleint64
values.cache/memory.go (2)
41-59
: Ensure atomicity and thread-safety ofDecrement
method.The
Decrement
method should ensure atomicity and thread-safety when decrementing values.Ensure that the
Decrement
method properly handles concurrent access and maintains atomicity.
141-159
: Ensure atomicity and thread-safety ofIncrement
method.The
Increment
method should ensure atomicity and thread-safety when incrementing values.Ensure that the
Increment
method properly handles concurrent access and maintains atomicity.go.mod (33)
56-56
: Verify the addition offilippo.io/edwards25519
Ensure that this new dependency is necessary and correctly integrated into the project.
63-63
: Verify the addition ofgithub.aaakk.us.kg/atotto/clipboard
Ensure that this new dependency is necessary and correctly integrated into the project.
65-65
: Verify the addition ofgithub.aaakk.us.kg/aymanbagabas/go-osc52/v2
Ensure that this new dependency is necessary and correctly integrated into the project.
67-67
: Verify the addition ofgithub.aaakk.us.kg/catppuccin/go
Ensure that this new dependency is necessary and correctly integrated into the project.
69-69
: Verify the addition ofgithub.aaakk.us.kg/charmbracelet/bubbles
Ensure that this new dependency is necessary and correctly integrated into the project.
70-70
: Verify the addition ofgithub.aaakk.us.kg/charmbracelet/bubbletea
Ensure that this new dependency is necessary and correctly integrated into the project.
71-71
: Verify the addition ofgithub.aaakk.us.kg/charmbracelet/x/ansi
Ensure that this new dependency is necessary and correctly integrated into the project.
72-72
: Verify the addition ofgithub.aaakk.us.kg/charmbracelet/x/exp/strings
Ensure that this new dependency is necessary and correctly integrated into the project.
73-73
: Verify the addition ofgithub.aaakk.us.kg/charmbracelet/x/input
Ensure that this new dependency is necessary and correctly integrated into the project.
74-74
: Verify the addition ofgithub.aaakk.us.kg/charmbracelet/x/term
Ensure that this new dependency is necessary and correctly integrated into the project.
75-75
: Verify the addition ofgithub.aaakk.us.kg/charmbracelet/x/windows
Ensure that this new dependency is necessary and correctly integrated into the project.
76-76
: Verify the addition ofgithub.aaakk.us.kg/containerd/console
Ensure that this new dependency is necessary and correctly integrated into the project.
80-80
: Verify the addition ofgithub.aaakk.us.kg/erikgeiser/coninput
Ensure that this new dependency is necessary and correctly integrated into the project.
81-81
: Verify the addition ofgithub.aaakk.us.kg/felixge/httpsnoop
Ensure that this new dependency is necessary and correctly integrated into the project.
83-83
: Verify the addition ofgithub.aaakk.us.kg/go-logr/logr
Ensure that this new dependency is necessary and correctly integrated into the project.
84-84
: Verify the addition ofgithub.aaakk.us.kg/go-logr/stdr
Ensure that this new dependency is necessary and correctly integrated into the project.
97-97
: Verify the addition ofgithub.aaakk.us.kg/gookit/color
Ensure that this new dependency is necessary and correctly integrated into the project.
105-105
: Verify the addition ofgithub.aaakk.us.kg/jackc/puddle/v2
Ensure that this new dependency is necessary and correctly integrated into the project.
113-113
: Verify the addition ofgithub.aaakk.us.kg/lithammer/fuzzysearch
Ensure that this new dependency is necessary and correctly integrated into the project.
114-114
: Verify the addition ofgithub.aaakk.us.kg/lucasb-eyer/go-colorful
Ensure that this new dependency is necessary and correctly integrated into the project.
117-117
: Verify the addition ofgithub.aaakk.us.kg/mattn/go-localereader
Ensure that this new dependency is necessary and correctly integrated into the project.
118-118
: Verify the addition ofgithub.aaakk.us.kg/mattn/go-runewidth
Ensure that this new dependency is necessary and correctly integrated into the project.
121-121
: Verify the addition ofgithub.aaakk.us.kg/muesli/ansi
Ensure that this new dependency is necessary and correctly integrated into the project.
122-122
: Verify the addition ofgithub.aaakk.us.kg/muesli/cancelreader
Ensure that this new dependency is necessary and correctly integrated into the project.
123-123
: Verify the addition ofgithub.aaakk.us.kg/muesli/termenv
Ensure that this new dependency is necessary and correctly integrated into the project.
127-127
: Verify the addition ofgithub.aaakk.us.kg/rabbitmq/amqp091-go
Ensure that this new dependency is necessary and correctly integrated into the project.
129-129
: Verify the addition ofgithub.aaakk.us.kg/rivo/uniseg
Ensure that this new dependency is necessary and correctly integrated into the project.
146-146
: Verify the addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc
Ensure that this new dependency is necessary and correctly integrated into the project.
147-147
: Verify the addition ofgo.opentelemetry.io/contrib/instrumentation/net/http/otelhttp
Ensure that this new dependency is necessary and correctly integrated into the project.
148-148
: Verify the addition ofgo.opentelemetry.io/otel
Ensure that this new dependency is necessary and correctly integrated into the project.
149-149
: Verify the addition ofgo.opentelemetry.io/otel/metric
Ensure that this new dependency is necessary and correctly integrated into the project.
150-150
: Verify the addition ofgo.opentelemetry.io/otel/trace
Ensure that this new dependency is necessary and correctly integrated into the project.
156-156
: Verify the addition ofgolang.org/x/term
Ensure that this new dependency is necessary and correctly integrated into the project.
cache/memory_test.go (19)
43-45
: Ensure correctness ofDecrement
method.The
Decrement
method now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
46-46
: Ensure correctness ofGetInt64
method.The
GetInt64
method is used to retrieve the decremented value. Verify that this method is correctly implemented and tested.
49-51
: Ensure correctness ofDecrement
method with parameter.The
Decrement
method with a parameter now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
53-55
: Ensure correctness ofDecrement
method with non-existent key.The
Decrement
method with a non-existent key now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
56-56
: Ensure correctness ofGetInt64
method.The
GetInt64
method is used to retrieve the decremented value for a non-existent key. Verify that this method is correctly implemented and tested.
58-58
: Ensure correctness ofDecrement
method with added value.The
Decrement
method with an added value now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
59-60
: Ensure correctness ofAdd
method withint64
value.The
Add
method now usesint64
values. Verify that all usages and test cases are correctly updated to reflect this change.
61-63
: Ensure correctness ofDecrement
method with added value.The
Decrement
method with an added value now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
65-66
: Ensure correctness ofDecrement
method with parameter.The
Decrement
method with a parameter now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
69-87
: Ensure correctness ofTestDecrementWithConcurrent
method.The
TestDecrementWithConcurrent
method now usesint64
values. Verify that all usages and test cases are correctly updated to reflect this change.
151-153
: Ensure correctness ofIncrement
method.The
Increment
method now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
154-154
: Ensure correctness ofGetInt64
method.The
GetInt64
method is used to retrieve the incremented value. Verify that this method is correctly implemented and tested.
157-159
: Ensure correctness ofIncrement
method with parameter.The
Increment
method with a parameter now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
161-163
: Ensure correctness ofIncrement
method with non-existent key.The
Increment
method with a non-existent key now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
164-164
: Ensure correctness ofGetInt64
method.The
GetInt64
method is used to retrieve the incremented value for a non-existent key. Verify that this method is correctly implemented and tested.
166-168
: Ensure correctness ofIncrement
method with added value.The
Increment
method with an added value now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
169-171
: Ensure correctness ofIncrement
method with added value.The
Increment
method with an added value now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
173-174
: Ensure correctness ofIncrement
method with parameter.The
Increment
method with a parameter now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
177-195
: Ensure correctness ofTestIncrementWithConcurrent
method.The
TestIncrementWithConcurrent
method now usesint64
values. Verify that all usages and test cases are correctly updated to reflect this change.mocks/cache/Driver.go (5)
Line range hint
77-89
: Ensure correctness ofDecrement
method in mock.The
Decrement
method now usesint64
. Verify that all usages and test cases are correctly updated to reflect this change.
91-92
: Ensure correctness ofDecrement
method return type.The
Decrement
method return type is nowint64
. Verify that all usages and test cases are correctly updated to reflect this change.
93-95
: Ensure correctness ofDecrement
method return type.The
Decrement
method return type is nowint64
. Verify that all usages and test cases are correctly updated to reflect this change.
96-98
: Ensure correctness ofDecrement
method return type.The
Decrement
method return type is nowint64
. Verify that all usages and test cases are correctly updated to reflect this change.
99-101
: Ensure correctness ofDecrement
method return type.The
Decrement
method return type is nowint64
. Verify that all usages and test cases are correctly updated to reflect this change.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #519 +/- ##
==========================================
- Coverage 69.98% 69.92% -0.07%
==========================================
Files 180 180
Lines 11025 11049 +24
==========================================
+ Hits 7716 7726 +10
- Misses 2738 2751 +13
- Partials 571 572 +1 ☔ View full report in Codecov by Sentry. |
@coderabbitai review |
Actions performedReview triggered.
|
@coderabbitai full review |
Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
go.sum
is excluded by!**/*.sum
mocks/cache/Cache.go
is excluded by!mocks/**
mocks/cache/Driver.go
is excluded by!mocks/**
Files selected for processing (6)
- cache/driver_test.go (2 hunks)
- cache/memory.go (6 hunks)
- cache/memory_test.go (3 hunks)
- contracts/cache/cache.go (2 hunks)
- go.mod (4 hunks)
- grpc/application.go (2 hunks)
Additional comments not posted (44)
contracts/cache/cache.go (2)
17-17
: Update method signature to useint64
for Decrement.The
Decrement
method signature has been updated to useint64
instead ofint
. This change is appropriate for atomic operations.
37-37
: Update method signature to useint64
for Increment.The
Increment
method signature has been updated to useint64
instead ofint
. This change is appropriate for atomic operations.grpc/application.go (1)
92-92
: Update UnaryServerInterceptors to use grpc.ChainUnaryInterceptor.The
UnaryServerInterceptors
method now usesgrpc.ChainUnaryInterceptor
for better handling of unary server interceptors. This change is appropriate and simplifies the setup.cache/driver_test.go (2)
82-82
: Update method signature to useint64
for Decrement.The
Decrement
method signature in theStore
struct has been updated to useint64
instead ofint
. This change is appropriate for atomic operations.
131-131
: Update method signature to useint64
for Increment.The
Increment
method signature in theStore
struct has been updated to useint64
instead ofint
. This change is appropriate for atomic operations.cache/memory.go (12)
5-7
: Add imports for sync and sync/atomic packages.The imports for
sync
andsync/atomic
packages are added. These are necessary for the newsync.Map
and atomic operations.
19-19
: Replace instance with sync.Map.The
instance
field in theMemory
struct is now async.Map
. This change improves concurrency performance.
24-24
: Set prefix in NewMemory constructor.The
prefix
field is set in theNewMemory
constructor. Ensure that theprefix
function is correctly defined and used.
30-33
: Update Add method to use sync.Map.The
Add
method is updated to usesync.Map
. TheLoadOrStore
method is used to add an item if the key does not exist.Also applies to: 36-37
40-59
: Refactor Decrement method for atomic operations.The
Decrement
method is refactored to use atomic operations withint64
. This change ensures thread-safe decrements.
64-67
: Update Forever method to use Put.The
Forever
method is updated to use thePut
method withNoExpiration
. This change ensures consistency in storing items indefinitely.
80-80
: Update Flush method to use sync.Map.The
Flush
method is updated to reset theinstance
field to a newsync.Map
. This change ensures thread-safe flushing of all items.
Line range hint
86-93
: Update Get method to use sync.Map.The
Get
method is updated to usesync.Map
. This change ensures thread-safe retrieval of items.
137-137
: Update Has method to use sync.Map.The
Has
method is updated to usesync.Map
. This change ensures thread-safe existence checks.
141-159
: Refactor Increment method for atomic operations.The
Increment
method is refactored to use atomic operations withint64
. This change ensures thread-safe increments.
181-187
: Update Put method to use sync.Map.The
Put
method is updated to usesync.Map
. This change ensures thread-safe storage of items.
224-224
: Update RememberForever method to use Put.The
RememberForever
method is updated to use thePut
method withNoExpiration
. This change ensures consistency in storing items indefinitely.go.mod (4)
56-56
: Dependency Addition: filippo.io/edwards25519The
filippo.io/edwards25519
package is a well-known library for elliptic curve cryptography. Ensure it is used securely and appropriately within the project.
63-63
: Dependency Addition: github.com/atotto/clipboardThe
github.com/atotto/clipboard
package allows for clipboard operations. Verify its necessity and usage within the project to avoid unnecessary dependencies.
65-65
: Dependency Addition: github.com/aymanbagabas/go-osc52/v2The
github.com/aymanbagabas/go-osc52/v2
package is used for OSC52 escape sequences. Confirm its purpose and usage within the project.
67-67
: Dependency Addition: github.com/catppuccin/goThe
github.com/catppuccin/go
package is a theme library. Ensure it is necessary and correctly integrated into the project.cache/memory_test.go (23)
5-5
: Import Addition: sync packageThe
sync
package is imported to handle concurrency. Ensure its usage is correct and necessary for the tests.
43-43
: Test Update: Use int64 for DecrementThe
Decrement
method is now tested withint64
values. Ensure this change is consistent with the method's updated implementation.
46-46
: Test Update: Use GetInt64The
GetInt64
method replacesGetInt
, aligning with the change toint64
. Ensure this method accurately retrieves the expected value.
49-49
: Test Update: Decrement with int64 parameterTesting the
Decrement
method with anint64
parameter. Ensure this test accurately reflects the method's updated functionality.
53-53
: Test Update: Decrement with int64 parameterTesting the
Decrement
method with anotherint64
parameter. Ensure this test accurately reflects the method's updated functionality.
56-56
: Test Update: Use GetInt64The
GetInt64
method replacesGetInt
, aligning with the change toint64
. Ensure this method accurately retrieves the expected value.
58-59
: Test Update: Add with int64 pointerAdding a value using an
int64
pointer. Ensure this test accurately reflects the method's updated functionality.
61-61
: Test Update: Decrement with int64Testing the
Decrement
method with anint64
parameter. Ensure this test accurately reflects the method's updated functionality.
65-66
: Test Update: Decrement with int64Testing the
Decrement
method with anotherint64
parameter. Ensure this test accurately reflects the method's updated functionality.
70-70
: Test Update: Use int64 for DecrementThe
Decrement
method is now tested withint64
values. Ensure this change is consistent with the method's updated implementation.
74-81
: Concurrency Test: Decrement with sync.WaitGroupTesting
Decrement
method with concurrent operations usingsync.WaitGroup
. Ensure this test accurately reflects the method's concurrency safety.
86-87
: Test Update: Use GetInt64The
GetInt64
method replacesGetInt
, aligning with the change toint64
. Ensure this method accurately retrieves the expected value.
151-151
: Test Update: Use int64 for IncrementThe
Increment
method is now tested withint64
values. Ensure this change is consistent with the method's updated implementation.
154-154
: Test Update: Use GetInt64The
GetInt64
method replacesGetInt
, aligning with the change toint64
. Ensure this method accurately retrieves the expected value.
157-157
: Test Update: Increment with int64 parameterTesting the
Increment
method with anint64
parameter. Ensure this test accurately reflects the method's updated functionality.
161-161
: Test Update: Increment with int64 parameterTesting the
Increment
method with anotherint64
parameter. Ensure this test accurately reflects the method's updated functionality.
164-164
: Test Update: Use GetInt64The
GetInt64
method replacesGetInt
, aligning with the change toint64
. Ensure this method accurately retrieves the expected value.
166-167
: Test Update: Add with int64 pointerAdding a value using an
int64
pointer. Ensure this test accurately reflects the method's updated functionality.
169-169
: Test Update: Increment with int64Testing the
Increment
method with anint64
parameter. Ensure this test accurately reflects the method's updated functionality.
173-174
: Test Update: Increment with int64Testing the
Increment
method with anotherint64
parameter. Ensure this test accurately reflects the method's updated functionality.
178-178
: Test Update: Use int64 for IncrementThe
Increment
method is now tested withint64
values. Ensure this change is consistent with the method's updated implementation.
182-189
: Concurrency Test: Increment with sync.WaitGroupTesting
Increment
method with concurrent operations usingsync.WaitGroup
. Ensure this test accurately reflects the method's concurrency safety.
194-195
: Test Update: Use GetInt64The
GetInt64
method replacesGetInt
, aligning with the change toint64
. Ensure this method accurately retrieves the expected value.
# Conflicts: # go.mod # go.sum
@coderabbitai review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
Files selected for processing (2)
- database/factory_test.go (1 hunks)
- go.mod (2 hunks)
Files skipped from review due to trivial changes (1)
- database/factory_test.go
Additional comments not posted (1)
go.mod (1)
9-9
: Verify compatibility ofgofakeit
version change.The version of
gofakeit
has been updated from v6 to v7. Ensure that the new version v7.0.4 is compatible with the existing codebase and does not introduce any breaking changes.
* feat: [#519] Save container to a temp file * add container * optimize docker * chore: update mocks * only generate one containter * remove * remove * fix ci * fix ci * remove * stop container * cancel test in windows * fix ci * fix ci * test ci * test ci * test ci * test ci * test ci * test ci * test ./database/... * move creating db process to NewTestQueries * create container directly * chore: update mocks * remove sqlite * normal * update config * fix postgres * optimize * cancel test * solve AI --------- Co-authored-by: hwbrzzl <[email protected]>
📑 Description
https://github.com/patrickmn/go-cache package use
sync.Mutex
, this PR replace it tosync.Map
for better performance.And also refactor
Increment
andDecrement
method toint64
for atom control (atomic package doesn't supportint
)✅ Checks
Summary by CodeRabbit
New Features
sync.Map
for better concurrency handling.int64
values, allowing for larger value ranges.Bug Fixes
Tests
int64
values.Chores