Improvements#87
Merged
Merged
Conversation
The UpdateCache goroutine used `return` instead of `continue` on error paths (empty database, query parse failure, table extraction failure). A single transient error would permanently kill the goroutine, stopping all server-response caching for the plugin's lifetime. Resolves #64.
IsCacheNeeded was called with the full cache key (server:db:request_bytes) instead of the actual SQL query string. Date/time function detection against raw binary bytes was unreliable. Now the query is parsed first via GetQueryFromRequest and uppercased before checking.
Move config field assignments (including ExitOnStartupError) before the API client initialization block so the flag is set when first checked. Also guard the deferred channel close to prevent a panic when config is nil and the channel was never initialized.
Introduce a dedicated CacheErrorsCounter to separate Redis operation failures from genuine cache misses. Previously CacheMissesCounter was incremented for SET/DEL/SCAN errors, making the metric unreliable. Also fix double-counting in OnTrafficFromClient where a single miss incremented CacheMissesCounter twice. Fix typos: cachedRespnseKey -> cachedResponseKey, DateFucntion -> DateFunction.
Replace three copies of the ExitOnStartupError check + manual close + os.Exit(1) pattern with a single handleStartupError helper that accepts variadic io.Closer resources. Reduces boilerplate and ensures consistent behavior across all startup error paths.
Previously cast.To* silently returned zero-values for invalid config. Now redisURL, expiry, and scanCount are validated with warnings and defaults applied when values are missing or invalid.
Add a WaitGroup to the Plugin struct so the UpdateCache goroutine signals completion when the channel is closed, allowing the process to drain pending cache updates before exit. Tests updated to use the same mechanism.
…rect deps SDK v0.4.4 brings redis/go-redis/v9, go-plugin v1.7.0, pgx/v5 v5.8.0, grpc v1.79.1, and protobuf v1.36.11. Also updates miniredis v2.36.1, sentry v0.42.0, cast v1.10.0, testify v1.11.1, and prometheus v1.23.2 via make update-direct-deps.
Replace deprecated github.com/go-redis/redis/v8 with the maintained github.com/redis/go-redis/v9. Updated import paths in plugin.go, main.go, plugin_test.go, and the depguard config in .golangci.yaml.
Bump actions/checkout to v4, actions/setup-go to v5 (Node 20), and softprops/action-gh-release to v2. Add govulncheck step to the test workflow for security scanning. setup-go v5 includes built-in module caching.
…esilience - TestIsCacheNeeded: table-driven test covering all PostgreSQL date/time functions plus positive cases (SELECT, INSERT, empty) - TestOnClosed: verifies client-to-database key cleanup on connection close - TestOnClosedNilClient: verifies graceful handling of nil client - TestInvalidateDML: verifies INSERT invalidates cached SELECT responses and their table index keys - TestInvalidateDMLSelectIgnored: verifies SELECT queries don't trigger invalidation - TestUpdateCacheContinuesOnError: confirms the goroutine survives errors and processes subsequent messages (validates the return->continue fix)
mostafa
force-pushed
the
improvements
branch
from
3ac13db to
2eee5d4
Compare
Sort imports alphabetically to satisfy gci linter. Reduce nestif complexity in UpdateCache by inverting the caching precondition into an early continue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket(s)
Resolves #64
Description
Bug fixes, code quality improvements, dependency updates, CI hardening, and expanded test coverage.
Bug fixes:
return->continueinUpdateCachegoroutine — a single transient error (e.g. empty database name) permanently killed all server-response caching for the plugin's lifetimeIsCacheNeededreceiving the raw cache key instead of the parsed SQL query stringExitOnStartupErrorbeing read before it was set inmain.goclose(nil)channel panic when config is nilCacheMissesCounterinOnTrafficFromClientCode quality:
CacheErrorsCounterto separate Redis operation errors from genuine cache misseshandleStartupErrorhelperredisURL,expiry,scanCountWaitGroupfor gracefulUpdateCachegoroutine shutdowncachedRespnseKey->cachedResponseKey,DateFucntion->DateFunctionDependencies:
gatewayd-plugin-sdkv0.4.3 -> v0.4.4,gatewaydv0.10.2 -> v0.11.0, and all direct depsgo-redis/redis/v8toredis/go-redis/v9CI:
actions/checkoutto v4,actions/setup-goto v5,softprops/action-gh-releaseto v2govulnchecksecurity scanning stepTests (coverage 50% -> 64%):
TestIsCacheNeeded— table-driven test for all PostgreSQL date/time functionsTestOnClosed/TestOnClosedNilClient— connection cleanupTestInvalidateDML/TestInvalidateDMLSelectIgnored— DML cache invalidationTestUpdateCacheContinuesOnError— validates goroutine resilience after thereturn->continuefixRelated PRs
None
Development Checklist
Legal Checklist