Implement BACKEND_TYPE=memcache as an alternative k/v store to redis #172

dweitzman · 2020-09-08T17:45:29Z

MEMCACHE_HOST_PORT=host:port must be set with BACKEND_TYPE=memcache

To minimize roundtrips when getting multiple keys, the memcache implementation
does a GetMulti to fetch the existing rate limit usage and does increments
asynchronously in background goroutines, since the memcache API doesn't offer
multi-increment.

Resolves #140

dweitzman · 2020-09-08T17:51:00Z

I'm open to whatever sort of feedback or changes here. I wish there wasn't so much duplication of unit test cases between redis and memcache, though avoiding that cleanly seems tricky.

MEMCACHE_HOST_PORT=host:port must be set with BACKEND_TYPE=memcache To minimize roundtrips when getting multiple keys, the memcache implementation does a GetMulti to fetch the existing rate limit usage and does increments asynchronously in background goroutines, since the memcache API doesn't offer multi-increment. Resolves envoyproxy#140 Signed-off-by: David Weitzman <dweitzman@pinterest.com>

mattklein123

Thanks for working on this. I didn't review the memcached code in detail, but structurally this LGTM. I had one small comment about the interface. Also, can you update the docs/README to discuss this feature, and also take this out of draft when it's ready for final review?

src/memcached/cache_impl.go

memcache: Add documentation to README Signed-off-by: David Weitzman <dweitzman@pinterest.com>

stale · 2020-09-27T12:45:56Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

stale · 2020-10-04T14:45:23Z

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

nezdolik · 2020-11-30T12:43:28Z

Hi @dweitzman @mattklein123 we at Spotify are also looking into memcache support for rate limiter service. Maybe we could sync effort for making this PR to be merged upstream?

dweitzman · 2020-11-30T18:09:58Z

Hey @nezdolik. The reason I hadn't sent this out more officially was that I was thinking about whether there was a way to refactor to minimize duplication in the test logic across redis and memcache. It's not the end of the world to have some duplicated test logic, I suppose, so it might be better to just land it first and then try to refactor.

(Another thing I'd like to do at some point is have the test cases themselves start a redis/memcache instance rather than having a bunch of them pre-started on different ports)

We're already using this memcache code in production for some services at Pinterest, incidentally.

@mattklein123 How do you feel about me mailing this is a non-draft as-is (after syncing and resolving merge conflicts) and then then trying in the future to see how practical it is to share test cases across the redis vs memcache tests?

mattklein123 · 2020-11-30T18:12:17Z

Sure I'm fine with that. @nezdolik can you help review? See also #193 which may have implications on this that need to be sorted out.

…erge

dweitzman · 2020-12-01T01:35:06Z

I updated the branch this PR was pointed at, but when a PR is "closed" a change to the branch doesn't seem to update a PR. Is it possible to re-open this PR somehow or should I create an entirely new one?

nezdolik · 2020-12-01T10:11:11Z

Sure thing, I can help with review. Maybe @mattklein123 has permissions to reopen this PR?

mattklein123 · 2020-12-01T17:45:43Z

Done!

dweitzman · 2020-12-01T18:07:04Z

I moved CalculateReset to the utils package so it could be shared across redis and memcache, and then to avoid a circular dependency I also moved time.go under utils.

dweitzman · 2020-12-01T19:43:45Z

Thanks for working on this. I didn't review the memcached code in detail, but structurally this LGTM. I had one small comment about the interface. Also, can you update the docs/README to discuss this feature, and also take this out of draft when it's ready for final review?

Updated readme

nezdolik

Will do one more pass tomorrow. Had some comments so far.

nezdolik · 2020-12-03T19:00:55Z

src/memcached/cache_impl.go

+	var err error
+
+	if len(keysToGet) > 0 {
+		memcacheValues, err = this.client.GetMulti(keysToGet)


Should we document somewhere the limitation of max key length (250 bytes)? https://github.com/bradfitz/gomemcache/blob/a41fca850d0b6f392931a78cbae438803ea0b886/memcache/memcache.go#L454

Added comments to cache_impl.go and README

nezdolik · 2020-12-03T19:13:07Z

src/memcached/cache_impl.go

+	logger.Debugf("starting cache lookup")
+
+	// request.HitsAddend could be 0 (default value) if not specified by the caller in the Ratelimit request.
+	hitsAddend := max(1, request.HitsAddend)


I see that bunch of code is duplicated across redis and memcache implementations particularly in this method. Would it be possible to factor that code our into some common method?

Note this comment is the same as this one: #193 (comment). It would be great to figure out some of this as I think the two PRs are going to run into very similar issues around duplication so I would take a look at my other comments in that one also.

ok, maybe we need a separate PR which pulls up duplicated code into single place

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

nezdolik · 2020-12-07T14:12:36Z

src/memcached/cache_impl.go

+	}
+
+	// Now fetch the pipeline.
+	responseDescriptorStatuses := make([]*pb.RateLimitResponse_DescriptorStatus,


Is pipeline term still applicable? (it is heavily used for redis, not aware how much for memcache)

nezdolik · 2020-12-07T14:13:07Z

src/memcached/cache_impl.go

+	expirationJitterMaxSeconds int64
+	cacheKeyGenerator          limiter.CacheKeyGenerator
+	localCache                 *freecache.Cache
+	wg                         sync.WaitGroup


nit: could expand field name to waitGroup

nezdolik · 2020-12-07T14:40:25Z

test/integration/integration_test.go

+	t.Run("WithoutPerSecondRedis", testBasicConfig("8083", "false", "0", ""))
+	t.Run("WithPerSecondRedis", testBasicConfig("8085", "true", "0", ""))
+	t.Run("WithoutPerSecondRedisWithLocalCache", testBasicConfig("18083", "false", "1000", ""))
+	t.Run("WithPerSecondRedisWithLocalCache", testBasicConfig("18085", "true", "1000", ""))


should there be also test for backend type "redis"?

"redis" is the default, but I modified TestBasicConfig so that some of the tests explicitly set "redis" instead of the empty string

nezdolik · 2020-12-07T14:45:55Z

test/memcached/cache_impl_test.go

+	"github.com/stretchr/testify/assert"
+)
+
+func TestMemcached(t *testing.T) {


as a matter of preference i would have split that test in 3 smaller ones

This is intended to exactly mirror the test cases in the redis version. I wanted to keep it similar so there's hope of potentially extracting the test cases into some common location to share between redis and memcache, although I'm still not sure exactly what that looks like.

nezdolik · 2020-12-07T14:49:01Z

test/memcached/cache_impl_test.go

+}
+
+func TestMemcacheWithJitter(t *testing.T) {
+	assert := assert.New(t)


every test sets up the same amount of routine code, this could be extracted to a sort of SetUp() function, to reduce the size of the test body and leave the essential part

nezdolik · 2020-12-07T14:57:55Z

test/memcached/cache_impl_test.go

+	"github.com/stretchr/testify/assert"
+)
+
+func TestMemcached(t *testing.T) {


would it be worth adding tests that simulate error responses from memcache backend?

Added TestMemcachedGetError which tests both an explicit error from memcache and no explicit error but a response that doesn't contain the expected key

nezdolik · 2020-12-14T18:56:18Z

@dweitzman i could take a stab at removing code duplication. Lmk if this is fine with you or if you would prefer to do it yourself.

dweitzman · 2020-12-14T20:01:48Z

@dweitzman i could take a stab at removing code duplication. Lmk if this is fine with you or if you would prefer to do it yourself.

If it gets done and I don't have to do it, I'm perfectly happy :)

Sorry I've been a bit slow updating this PR for to address your comments. I'll try to get an update by Wednesday night.

nezdolik · 2020-12-16T20:50:02Z

Thanks @dweitzman. I will be able to help out with duplicated code next week.

- Add memcache error unit test - Rename wg -> waitGroup - Use explict "redis" for backend type in some tests instead of always assume it's the default Signed-off-by: David Weitzman <dweitzman@pinterest.com>

dweitzman

Addressed some of the review comments. The force push is because I realized immediately after pushing that my commit didn't have a "Signed-off-by", but no previous history before the new commit is modified

nezdolik · 2020-12-28T16:37:05Z

@dweitzman started refactoring of duplicated code here: nezdolik@1ff507a (it's based on latest state from your branch). Planning to finish it tomorrow and then we could add those commits to this PR.

nezdolik · 2020-12-29T15:28:05Z

src/memcached/cache_impl.go

+		if ok {
+			decoded, err := strconv.ParseInt(string(rawMemcacheValue.Value), 10, 32)
+			if err != nil {
+				logger.Errorf("Unexpected non-numeric value in memcached: %v", rawMemcacheValue)


should there be continue keyword here? Does it make sense to go further and calculate limits with default initialised uint32?

Seems like this case would still benefit from the calculation. An extreme example would be if hitsAdded was high and the limit was lower.

Or someone might also set a quota of 0 if they wanted to stop traffic entirely, and we'd want to respect that even if memcache didn't return anything

nezdolik · 2020-12-29T17:46:02Z

@dweitzman @mattklein123 finished code refactoring, need to add unit tests for new code. I could also create a temp PR if it would be more convenient to review. Please take a look (it's my first Go code): nezdolik@5842b4b

nezdolik · 2020-12-30T16:12:44Z

Added tests to refactoring code, it is now complete. It may be easier to merge existing PR and then refactoring PR on top of it. Created refactoring PR here: #202, will rebase it on master once this PR is merged.

mattklein123

SGTM to merge and then refactor. Thanks!

* Add Docker Compose File (#27) * docs: match envoy docs for remote_address ratelimiting (#29) * docs: document dependency on gostats (#30) * test and document whitelist behavior (#31) Signed-off-by: Daniel Hochman <danielhochman@users.noreply.github.com> * update dependencies (#35) * update dependencies * proto: use the proto defined in data-plane-api (#39) * proto: check in protos to allow importing ratelimit as a library (#40) * docs: update contact info (#42) * redis: add the option to use a separate redis pool for per second limits (#41) * fix duplicate mv (#43) * docker: upgrade docker-compose setup (#46) * Add gRPC health check (#47) * logging: set log level (#50) * go version: update to 1.11 (#53) * docker compose: expose gRPC port on docker compose setup (#55) * Configuration to ignore dotfiles. (#52) This allows ratelimit to run on Kubernetes with configuration from a configmap. * Add Dockerfile to enable builds (#58) Signed-off-by: Steve Sloka <steves@heptio.com> * ci: fix build (#73) Fixes envoyproxy#71 Signed-off-by: Matt Klein <mklein@lyft.com> * Run unit and integration tests with race detector enabled (#65) * deps: update several of ratelimit's dependencies (#76) * add stalebot (#78) Signed-off-by: Matt Klein <mklein@lyft.com> * docs: fix example 4 sample config (#79) * fix redis-server binary name (envoyproxy#88) * Fix build Dockerfile (envoyproxy#98) Fix problem: src/service_cmd/runner/runner.go:10:2: cannot find package "github.com/lyft/ratelimit/proto/ratelimit" in any of: /usr/local/go/src/github.com/lyft/ratelimit/vendor/github.com/lyft/ratelimit/proto/ratelimit (vendor tree) /usr/local/go/src/vendor/github.com/lyft/ratelimit/proto/ratelimit /usr/local/go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOROOT) /go/src/github.com/lyft/ratelimit/proto/ratelimit (from $GOPATH) The command '/bin/sh -c go build -o /usr/local/bin/ratelimit src/service_cmd/main.go' returned a non-zero code: 1 * Redis TLS and Auth support (envoyproxy#96) This adds support for TLS connections to redis as well as support for authentication. Somewhat related to issue #61 * healthcheck: allow customizable healthcheck name (envoyproxy#102) Description: this patch allows a consumer of the server package to customize the name of the healthchecker. Signed-off-by: Jose Nino <jnino@lyft.com> * health: make a few more types public (envoyproxy#104) Description: envoyproxy#102 allowed for some customization. This PR makes the types public so that other servers can use this implementation. Signed-off-by: Jose Nino <jnino@lyft.com> * Add local cache to store whether it is over the limit (envoyproxy#111) * Plugin statstore into runner (envoyproxy#115) * fix: support auth without tls (envoyproxy#116) Signed-off-by: tangxinfa <tangxinfa@gmail.com> * add local cache stats (envoyproxy#114) * Move license to templated Apache-2.0 (envoyproxy#123) Signed-off-by: Derek Schaller <d_a_schaller@yahoo.com> * Enable go modules (envoyproxy#124) Signed-off-by: Steve Sloka <slokas@vmware.com> * CI: Github Actions (envoyproxy#127) Signed-off-by: Steve Sloka <slokas@vmware.com> * community: update contributing guide (envoyproxy#139) Fixes envoyproxy#138 Signed-off-by: Matt Klein <mklein@lyft.com> * add http 1 `/json` endpoint (envoyproxy#136) Signed-off-by: David Black <david.black@autodesk.com> * Use mockgen version from go.mod instead of from "make bootstrap" (envoyproxy#143) Even though the Makefile wants to encourage using mockgen@1.4.1, it seems like the mocks have been generated using a pre-1.0 version of mockgen. Using "go run github.com/golang/mock/mockgen" as a go:generate command instead of just "mockgen" avoids the need to pre-install into the developer's $PATH and uses the go.mod-specified version Signed-off-by: David Weitzman <dweitzman@pinterest.com> * Upgrade gostats dependency from 0.2.6 to 0.4.0 (envoyproxy#141) My interest is the UDP protocol support which appeared in gotstats 0.3.10 There's a breaking change as of https://github.com/lyft/gostats/releases/tag/v0.3.0 which is that gostats no longer publishes stats as expvars. Signed-off-by: David Weitzman <dweitzman@pinterest.com> * Upgrade radix (envoyproxy#137) Signed-off-by: Tong Cai <caitong93@gmail.com> * cache_impl_test.go: fix failing test with ipv6 (envoyproxy#144) A newly-added test in envoyproxy#137 checks the exact text of an error message which seems to vary when the network is tcp4 vs tcp6. This change relaxes the assertion to look for "connection refused" in a panic without making assumptions about what an IP address looks like. Example failure: --- FAIL: TestNewClientImpl (0.00s) --- FAIL: TestNewClientImpl/connection_refused (0.00s) cache_impl_test.go:442: Error Trace: cache_impl_test.go:442 Error: func (assert.PanicTestFunc)(0x1724110) should panic with error message: "dial tcp 127.0.0.1:12345: connect: connection refused" Panic value: "dial tcp [::1]:12345: connect: connection refused" Panic stack: goroutine 27 [running]: The testify assert package doesn't seem to support inexact matching on error messages, so the code gets a bit uglier than before. Signed-off-by: David Weitzman <dweitzman@pinterest.com> * Split redis-specific logic from generic key-value store logic (envoyproxy#142) This is a pure refactoring with no behavior changes. It's a step toward being able to add memcache as a backend (see envoyproxy#140). This PR moves RateLimitCache from the redis package to a new "limiter" package, along with code for time/jitter, local cache stats, and constructing cache keys. All that can be reused with memcache. After this PR, the redis package is imported in exactly two places: - in service_cmd/runner/runner.go to call redis.NewRateLimiterCacheImplFromSettings() - in service/ratelimit.go in ShouldRateLimit to identify if a recovered panic is a redis.RedisError. If so, a stat is incremented and the panic() propagation is ended and in favor of returning the error as a the function result. The PR also includes changes by goimports to test/service/ratelimit_test.go so that the difference between package name vs file path name is explicit instead of implicit. Signed-off-by: David Weitzman <dweitzman@pinterest.com> * json handler: return full ratelimit service response as json (envoyproxy#148) Previously an HTTP POST to /json would only return an HTTP status code, not all the other details supported by grpc ratelimit responses. With this change an HTTP POST to /json receives the full proto3 response encoded as json by jsonpb. It seems unlikely that anyone would be parsing the text "over limit" from the HTTP body instead of just reading the 429 response code, but for anyone doing that this would be a breaking change. Signed-off-by: David Weitzman <dweitzman@pinterest.com> * Update goruntime to latest, 0.2.5. Add new config for watching changes in runtime config folder directly instead of the runtime root dir. (envoyproxy#151) Signed-off-by: Yuki Sawa <yukisawa@gmail.com> * Drop support for legacy ratelimit.proto and upgrade to v3 rls.proto (envoyproxy#153) Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org> * Followups to v3 upgrade (envoyproxy#155) - Regenerate mocks based on new default protocol - Manually transform v2 messages to v3 messages - some of the fields were renamed thus json Marshal/Unmarshal does not work anymore - Added tests that verify conversion v2<->v3 works for headers fields - Update tests to use proto.Equal - simple assert.Equals might not work correctly for protobuf messages. Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org> * Introduce a Dockerfile for running integration tests (envoyproxy#156) This diff creates Dockerfile.integration for running integration tests with clearly-defined dependencies. Previously the dependencies of the integration tests were defined within the github actions config. The new "make docker_tests" target should work for any developer with Docker installed. Previously there was no single command that would run integration tests across platforms, which makes development and onboarding harder. Even copying the command from github actions wouldn't have worked before, since that command quietly assumed that redis was already running on port 6379. Signed-off-by: David Weitzman <dweitzman@pinterest.com> * Add support for rate limit overrides. (envoyproxy#158) Fixes envoyproxy#154 Signed-off-by: Petr Pchelko <ppchelko@wikimedia.org> * redis client: default to use explicit pipelining (envoyproxy#163) Signed-off-by: Tong Cai <caitong93@gmail.com> * Clean go.mod file and update logrus to latest (envoyproxy#166) Signed-off-by: Yuki Sawa <yukisawa@gmail.com> * Add full test environment example. Fix bug in existing docker-compose. (envoyproxy#170) Signed-off-by: Yuki Sawa <yukisawa@gmail.com> * Implement LOG_FORMAT=json (envoyproxy#173) Centralized log collection system works better with logs in json format. E.g. DataDog strongly encourage setting up your logging library to produce your logs in JSON format to avoid the need for custom parsing rules. So, the next small fix is all we need to get json logs. Signed-off-by: Sergey Belyaev <sbelyaev@setronica.com> * ci: Update github action to push docker image tagged with sha for each merge to master branch (envoyproxy#176) Updates the github action to also push a tagged image based upon the git sha. The tag also includes the current version of the release. Example tag: envoyproxy/ratelimit:f1758150b6dfed3e5c0ae13fb7bb6b8f6ae00b0e Fixes envoyproxy#174 Signed-off-by: Steve Sloka <slokas@vmware.com> * Update README.md to refer to existing files (envoyproxy#178) Signed-off-by: Margaret Gorguissian <margaret.gorguissian@tufts.edu> * Add redis cluster and sentinel support (envoyproxy#179) Signed-off-by: Diego Erdody <diego@medallia.com> * Add support for x-ratelimit-reset header (envoyproxy#182) Signed-off-by: Clara Andrew-Wani <candrewwani@gmail.com> * Create repokitteh.star (envoyproxy#187) Signed-off-by: Itay Donanhirsh <itay@bazoo.org> * refactor NearLimitRatio to environment variable (envoyproxy#186) Signed-off-by: zufardhiyaulhaq <zufardhiyaulhaq@gmail.com> * Fix flakey tests with DurationUntilReset. Update docker example to V3 config. (envoyproxy#192) Signed-off-by: Yuki Sawa <yukisawa@gmail.com> * Separate Redis cache and driver implementation (envoyproxy#194) Signed-off-by: William Albertus Dembo <w.albertusd@gmail.com> * Set ratelimit filter to v3 api (envoyproxy#196) Signed-off-by: Yuki Sawa <yukisawa@gmail.com> * Add debug logging to indicate descriptor and limit (envoyproxy#197) Signed-off-by: Sasha Kulbii <okulbii@wayfair.com> * Implement BACKEND_TYPE=memcache as an alternative k/v store to redis (envoyproxy#172) MEMCACHE_HOST_PORT=host:port must be set with BACKEND_TYPE=memcache To minimize roundtrips when getting multiple keys, the memcache implementation does a GetMulti to fetch the existing rate limit usage and does increments asynchronously in background goroutines, since the memcache API doesn't offer multi-increment. Resolves envoyproxy#140 Signed-off-by: David Weitzman <dweitzman@pinterest.com> * Refactoring of duplicated code across backend types (envoyproxy#202) Signed-off-by: Kateryna Nezdolii <nezdolik@spotify.com> * Small typo fix in README (envoyproxy#204) Signed-off-by: cpaika <paika.christopher@gmail.com> * Circle CI (#1) * Add Circle Config * Touch to build * Move to expected path * Use Docker Command * Do it all here then * Env * Add Readme * Actual README * Add Docker Compose (#2) * VAULT-893 Fix docker-compose.yml * VAULT-893 Fix docker-compose.yml Co-authored-by: Marshall Jones <marshall@offby3.com> Co-authored-by: Jose Ulises Nino Rivera <junr03@users.noreply.github.com> Co-authored-by: Daniel Hochman <danielhochman@users.noreply.github.com> Co-authored-by: Martien Verbruggen <martien.verbruggen@gmail.com> Co-authored-by: Ben Pope <BenPope@users.noreply.github.com> Co-authored-by: Steve Sloka <steve@stevesloka.com> Co-authored-by: Matt Klein <mattklein123@gmail.com> Co-authored-by: Charlie Vieth <charlie.vieth@gmail.com> Co-authored-by: Adil Hafeez <ahafeez@lyft.com> Co-authored-by: Kartograf <kartogrof@gmail.com> Co-authored-by: repl-david-winiarski <33431229+repl-david-winiarski@users.noreply.github.com> Co-authored-by: Junchao Lyu <6963707+freedomljc@users.noreply.github.com> Co-authored-by: tangxinfa <tangxinfa@gmail.com> Co-authored-by: Steve Sloka <slokas@vmware.com> Co-authored-by: Matt Klein <mklein@lyft.com> Co-authored-by: dblackdblack <github@dhb.is> Co-authored-by: David Weitzman <dweitzman@pinterest.com> Co-authored-by: Tong Cai <caitong@caicloud.io> Co-authored-by: Yuki Sawa <yukisawa@gmail.com> Co-authored-by: Petr Pchelko <petrpchelko@gmail.com> Co-authored-by: Petr Pchelko <ppchelko@wikimedia.org> Co-authored-by: Sergey Belyaev <svdba@users.noreply.github.com> Co-authored-by: Margaret G <Margaret.Gorguissian@tufts.edu> Co-authored-by: Diego Erdody <erdody@gmail.com> Co-authored-by: Clara <candrewwani@gmail.com> Co-authored-by: Itay Donanhirsh <itay@bazoo.org> Co-authored-by: Zufar Dhiyaulhaq <zufardhiyaulhaq@gmail.com> Co-authored-by: William Albertus Dembo <w.albertusd@gmail.com> Co-authored-by: Alex Kulbii <jncneo@gmail.com> Co-authored-by: Kateryna Nezdolii <nezdolik@spotify.com> Co-authored-by: Christopher <paika.christopher@gmail.com>

…nvoyproxy#172) MEMCACHE_HOST_PORT=host:port must be set with BACKEND_TYPE=memcache To minimize roundtrips when getting multiple keys, the memcache implementation does a GetMulti to fetch the existing rate limit usage and does increments asynchronously in background goroutines, since the memcache API doesn't offer multi-increment. Resolves envoyproxy#140 Signed-off-by: David Weitzman <dweitzman@pinterest.com>

dweitzman force-pushed the memcache_impl branch from 573eed6 to c9784b6 Compare September 8, 2020 17:46

dweitzman force-pushed the memcache_impl branch 2 times, most recently from 9f7267f to 0478e41 Compare September 8, 2020 17:53

dweitzman force-pushed the memcache_impl branch from 0478e41 to 0a76b0b Compare September 8, 2020 17:55

mattklein123 self-assigned this Sep 8, 2020

mattklein123 requested changes Sep 9, 2020

View reviewed changes

src/memcached/cache_impl.go Outdated Show resolved Hide resolved

memcache: pull Flush() up to general RateLimitCache interface

f8f7de4

memcache: Add documentation to README Signed-off-by: David Weitzman <dweitzman@pinterest.com>

dweitzman force-pushed the memcache_impl branch from 362f6b2 to f8f7de4 Compare September 18, 2020 18:38

stale bot added the stale label Sep 27, 2020

stale bot closed this Oct 4, 2020

Merge remote-tracking branch 'opensource/master' into memcache_impl_m…

06a7f0c

…erge

mattklein123 reopened this Dec 1, 2020

stale bot removed the stale label Dec 1, 2020

dweitzman marked this pull request as ready for review December 1, 2020 17:58

mattklein123 assigned nezdolik Dec 2, 2020

mattklein123 added the waiting:any label Dec 3, 2020

nezdolik reviewed Dec 3, 2020

View reviewed changes

repokitteh-read-only bot removed the waiting:any label Dec 3, 2020

Document memcache max key length

7e68912

Signed-off-by: David Weitzman <dweitzman@pinterest.com>

nezdolik reviewed Dec 7, 2020

View reviewed changes

Address some review comments for memcached support:

fcb9523

- Add memcache error unit test - Rename wg -> waitGroup - Use explict "redis" for backend type in some tests instead of always assume it's the default Signed-off-by: David Weitzman <dweitzman@pinterest.com>

dweitzman force-pushed the memcache_impl branch from 975b5f3 to fcb9523 Compare December 24, 2020 00:08

dweitzman commented Dec 24, 2020

View reviewed changes

mattklein123 added the waiting label Dec 28, 2020

nezdolik reviewed Dec 29, 2020

View reviewed changes

nezdolik mentioned this pull request Dec 30, 2020

Refactoring of duplicated code across backend types #202

Merged

nezdolik approved these changes Dec 30, 2020

View reviewed changes

mattklein123 approved these changes Jan 2, 2021

View reviewed changes

mattklein123 merged commit 034c4ab into envoyproxy:master Jan 2, 2021

nezdolik mentioned this pull request Jan 7, 2021

Rolling window limit support #193

Closed

Implement BACKEND_TYPE=memcache as an alternative k/v store to redis #172

Implement BACKEND_TYPE=memcache as an alternative k/v store to redis #172

Conversation

dweitzman commented Sep 8, 2020

dweitzman commented Sep 8, 2020

mattklein123 left a comment

Choose a reason for hiding this comment

stale bot commented Sep 27, 2020

stale bot commented Oct 4, 2020

nezdolik commented Nov 30, 2020

dweitzman commented Nov 30, 2020 • edited Loading

mattklein123 commented Nov 30, 2020

dweitzman commented Dec 1, 2020 • edited Loading

nezdolik commented Dec 1, 2020

mattklein123 commented Dec 1, 2020

dweitzman commented Dec 1, 2020

dweitzman commented Dec 1, 2020

nezdolik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nezdolik commented Dec 14, 2020

dweitzman commented Dec 14, 2020

nezdolik commented Dec 16, 2020 • edited Loading

dweitzman left a comment • edited Loading

Choose a reason for hiding this comment

nezdolik commented Dec 28, 2020

nezdolik Dec 29, 2020 • edited Loading

Choose a reason for hiding this comment

dweitzman Dec 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nezdolik commented Dec 29, 2020 • edited Loading

nezdolik commented Dec 30, 2020 • edited Loading

mattklein123 left a comment

Choose a reason for hiding this comment

dweitzman commented Nov 30, 2020 •

edited

Loading

dweitzman commented Dec 1, 2020 •

edited

Loading

nezdolik commented Dec 16, 2020 •

edited

Loading

dweitzman left a comment •

edited

Loading

nezdolik Dec 29, 2020 •

edited

Loading

dweitzman Dec 29, 2020 •

edited

Loading

nezdolik commented Dec 29, 2020 •

edited

Loading

nezdolik commented Dec 30, 2020 •

edited

Loading