From 0adecb2127252caae3606023add9513895fb728f Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 24 Mar 2020 17:41:22 +0100 Subject: [PATCH 1/2] Remove all references to Redis It wasn't used, it was only adding complexity and causing test failures. --- .circleci/config.yml | 5 +- DEVELOPING.md | 1 - go.list | 1 - go.mod | 1 - go.sum | 2 - services/horizon/cmd/root.go | 13 ---- services/horizon/internal/app.go | 5 -- services/horizon/internal/config.go | 2 - services/horizon/internal/docs/admin.md | 2 - .../internal/docs/notes_for_developers.md | 8 +-- services/horizon/internal/init.go | 60 +------------------ services/horizon/internal/init_test.go | 33 ---------- services/horizon/internal/middleware_test.go | 30 ---------- 13 files changed, 5 insertions(+), 158 deletions(-) delete mode 100644 services/horizon/internal/init_test.go diff --git a/.circleci/config.yml b/.circleci/config.yml index d1e2231679..7f2f7b47aa 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -108,10 +108,9 @@ commands: environment: DOCKERIZE_VERSION: v0.3.0 - run: - name: Wait for postgres and redis + name: Wait for postgres command: | dockerize -wait tcp://localhost:5432 -timeout 1m - dockerize -wait tcp://localhost:6379 -timeout 1m - run: name: Run package tests environment: @@ -172,7 +171,6 @@ jobs: - image: circleci/postgres:9.6.5-alpine-ram environment: POSTGRES_USER: circleci - - image: circleci/redis:5.0-alpine steps: - install_go_deps - test_packages @@ -193,7 +191,6 @@ jobs: - image: circleci/postgres:9.6.5-alpine-ram environment: POSTGRES_USER: circleci - - image: circleci/redis:5.0-alpine steps: - install_go_deps - test_packages diff --git a/DEVELOPING.md b/DEVELOPING.md index dc8ce5d33c..e7980c0325 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -16,7 +16,6 @@ To checkout, build, and run most tests these tools are required: To run some tests these tools are also required: - PostgreSQL 9.6+ server running locally, or set [environment variables](https://www.postgresql.org/docs/9.6/libpq-envars.html) (e.g. `PGHOST`, etc) for alternative host. - MySQL 10.1+ server running locally. -- Redis 5.0+ server running on localhost port 6379. ## Get the code diff --git a/go.list b/go.list index 68e3371b9f..764803325e 100644 --- a/go.list +++ b/go.list @@ -42,7 +42,6 @@ github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b github.com/golang/mock v1.1.1 github.com/golang/protobuf v1.3.1 github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db -github.com/gomodule/redigo v2.0.0+incompatible github.com/google/go-cmp v0.2.0 github.com/google/go-querystring v0.0.0-20160401233042-9235644dd9e5 github.com/google/martian v2.1.0+incompatible diff --git a/go.mod b/go.mod index 085e3370ad..a1f680ba12 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,6 @@ require ( github.com/go-errors/errors v0.0.0-20150906023321-a41850380601 github.com/gobuffalo/packr v1.12.1 // indirect github.com/goji/httpauth v0.0.0-20160601135302-2da839ab0f4d - github.com/gomodule/redigo v2.0.0+incompatible github.com/google/go-querystring v0.0.0-20160401233042-9235644dd9e5 // indirect github.com/google/martian v2.1.0+incompatible // indirect github.com/googleapis/gax-go v2.0.2+incompatible // indirect diff --git a/go.sum b/go.sum index 6a0ac5bf56..74bc7e50cf 100644 --- a/go.sum +++ b/go.sum @@ -73,8 +73,6 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/golang/protobuf v1.3.1 h1:YF8+flBXS5eO826T4nzqPrxfhQThhXl0YzfuUPu4SBg= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= -github.com/gomodule/redigo v2.0.0+incompatible h1:K/R+8tc58AaqLkqG2Ol3Qk+DR/TlNuhuh457pBFPtt0= -github.com/gomodule/redigo v2.0.0+incompatible/go.mod h1:B4C85qUVwatsJoIUNIfCRsp7qO0iAmpGFZ4EELWSbC4= github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-querystring v0.0.0-20160401233042-9235644dd9e5 h1:oERTZ1buOUYlpmKaqlO5fYmz8cZ1rYu5DieJzF4ZVmU= diff --git a/services/horizon/cmd/root.go b/services/horizon/cmd/root.go index 596eac7195..302cd5fde9 100644 --- a/services/horizon/cmd/root.go +++ b/services/horizon/cmd/root.go @@ -218,18 +218,6 @@ var configOpts = support.ConfigOptions{ }, Usage: "max count of requests allowed in a one hour period, by remote ip address", }, - &support.ConfigOption{ - Name: "rate-limit-redis-key", - ConfigKey: &config.RateLimitRedisKey, - OptType: types.String, - Usage: "redis key for storing rate limit data, useful when deploying a cluster of Horizons, ignored when redis-url is empty", - }, - &support.ConfigOption{ - Name: "redis-url", - ConfigKey: &config.RedisURL, - OptType: types.String, - Usage: "redis to connect with, for rate limiting", - }, &support.ConfigOption{ Name: "friendbot-url", ConfigKey: &config.FriendbotURL, @@ -395,7 +383,6 @@ func initRootConfig() { // Validate options that should be provided together validateBothOrNeither("tls-cert", "tls-key") - validateBothOrNeither("rate-limit-redis-key", "redis-url") // config.HistoryArchiveURLs contains a single empty value when empty so using // viper.GetString is easier. diff --git a/services/horizon/internal/app.go b/services/horizon/internal/app.go index 9863fb3755..97f2e32dda 100644 --- a/services/horizon/internal/app.go +++ b/services/horizon/internal/app.go @@ -10,7 +10,6 @@ import ( "sync" "time" - "github.com/gomodule/redigo/redis" metrics "github.com/rcrowley/go-metrics" "github.com/stellar/go/clients/stellarcore" "github.com/stellar/go/exp/orderbook" @@ -40,7 +39,6 @@ type App struct { coreQ *core.Q ctx context.Context cancel func() - redis *redis.Pool coreVersion string horizonVersion string currentProtocolVersion int32 @@ -503,9 +501,6 @@ func (a *App) init() { // txsub.metrics initTxSubMetrics(a) - - // redis - initRedis(a) } // run is the function that runs in the background that triggers Tick each diff --git a/services/horizon/internal/config.go b/services/horizon/internal/config.go index 15f5a01c59..334504b54f 100644 --- a/services/horizon/internal/config.go +++ b/services/horizon/internal/config.go @@ -28,8 +28,6 @@ type Config struct { SSEUpdateFrequency time.Duration ConnectionTimeout time.Duration RateQuota *throttled.RateQuota - RateLimitRedisKey string - RedisURL string FriendbotURL *url.URL LogLevel logrus.Level LogFile string diff --git a/services/horizon/internal/docs/admin.md b/services/horizon/internal/docs/admin.md index 90b5a87425..5838ca8b54 100644 --- a/services/horizon/internal/docs/admin.md +++ b/services/horizon/internal/docs/admin.md @@ -20,8 +20,6 @@ The Stellar Development Foundation runs two Horizon servers, one for the public Horizon is dependent upon a stellar-core server. Horizon needs access to both the SQL database and the HTTP API that is published by stellar-core. See [the administration guide](https://www.stellar.org/developers/stellar-core/learn/admin.html ) to learn how to set up and administer a stellar-core server. Secondly, Horizon is dependent upon a postgres server, which it uses to store processed core data for ease of use. Horizon requires postgres version >= 9.5. -In addition to the two prerequisites above, you may optionally install a redis server to be used for rate limiting requests. - ## Installing To install Horizon, you have a choice: either downloading a [prebuilt release for your target architecture](https://github.com/stellar/go/releases) and operation system, or [building Horizon yourself](#Building). When either approach is complete, you will find yourself with a directory containing a file named `horizon`. This file is a native binary. diff --git a/services/horizon/internal/docs/notes_for_developers.md b/services/horizon/internal/docs/notes_for_developers.md index 6b42db1da5..9528313825 100644 --- a/services/horizon/internal/docs/notes_for_developers.md +++ b/services/horizon/internal/docs/notes_for_developers.md @@ -85,13 +85,7 @@ Check existing tests for more examples. ## Running Tests -start a redis server on port `6379` - -```bash -redis-server -``` - -then, run the all the Go monorepo tests like so (assuming you are at stellar/go, or run from stellar/go/services/horizon for just the Horizon subset): +run the all the Go monorepo tests like so (assuming you are at stellar/go, or run from stellar/go/services/horizon for just the Horizon subset): ```bash go test ./... diff --git a/services/horizon/internal/init.go b/services/horizon/internal/init.go index 6a463de8e9..ef77173ccb 100644 --- a/services/horizon/internal/init.go +++ b/services/horizon/internal/init.go @@ -3,12 +3,10 @@ package horizon import ( "context" "net/http" - "net/url" - "time" - raven "github.com/getsentry/raven-go" - "github.com/gomodule/redigo/redis" - metrics "github.com/rcrowley/go-metrics" + "github.com/getsentry/raven-go" + "github.com/rcrowley/go-metrics" + "github.com/stellar/go/exp/orderbook" "github.com/stellar/go/services/horizon/internal/db2/core" "github.com/stellar/go/services/horizon/internal/db2/history" @@ -146,58 +144,6 @@ func initWebMetrics(app *App) { app.metrics.Register("requests.failed", app.web.failureMeter) } -func initRedis(app *App) { - if app.config.RedisURL == "" { - return - } - - redisURL, err := url.Parse(app.config.RedisURL) - if err != nil { - log.Fatal(err) - } - - app.redis = &redis.Pool{ - MaxIdle: 3, - IdleTimeout: 240 * time.Second, - Dial: dialRedis(redisURL), - TestOnBorrow: func(c redis.Conn, t time.Time) error { - _, pingErr := c.Do("PING") - return pingErr - }, - } - - // test the connection - c := app.redis.Get() - defer c.Close() - - _, err = c.Do("PING") - if err != nil { - log.Fatal(err) - } -} - -func dialRedis(redisURL *url.URL) func() (redis.Conn, error) { - return func() (redis.Conn, error) { - c, err := redis.Dial("tcp", redisURL.Host) - if err != nil { - return nil, err - } - - if redisURL.User == nil { - return c, err - } - - if pass, ok := redisURL.User.Password(); ok { - if _, err = c.Do("AUTH", pass); err != nil { - c.Close() - return nil, err - } - } - - return c, err - } -} - func initSubmissionSystem(app *App) { // Due to a delay between Stellar-Core closing a ledger and Horizon // ingesting it, it's possible that results of transaction submitted to diff --git a/services/horizon/internal/init_test.go b/services/horizon/internal/init_test.go deleted file mode 100644 index 2b648414d2..0000000000 --- a/services/horizon/internal/init_test.go +++ /dev/null @@ -1,33 +0,0 @@ -package horizon - -import ( - "testing" - - "github.com/gomodule/redigo/redis" - "github.com/stretchr/testify/assert" -) - -func TestInitRedis(t *testing.T) { - c := NewTestConfig() - - // app.redis is nil when no RedisURL is set - c.RedisURL = "" - app := NewApp(c) - assert.Nil(t, app.redis) - app.Close() - - // app.redis gets set when RedisURL is set - c.RedisURL = "redis://127.0.0.1:6379/" - app = NewApp(c) - assert.NotNil(t, app.redis) - - // redis connection works - conn := app.redis.Get() - conn.Do("SET", "hello", "World") - world, _ := redis.String(conn.Do("GET", "hello")) - - assert.Equal(t, "World", world) - - conn.Close() - app.Close() -} diff --git a/services/horizon/internal/middleware_test.go b/services/horizon/internal/middleware_test.go index 409fa037a1..2e694b3c02 100644 --- a/services/horizon/internal/middleware_test.go +++ b/services/horizon/internal/middleware_test.go @@ -122,36 +122,6 @@ func TestRateLimitMiddlewareTestSuite(t *testing.T) { suite.Run(t, new(RateLimitMiddlewareTestSuite)) } -// Rate Limiting works with redis -func TestRateLimit_Redis(t *testing.T) { - ht := StartHTTPTest(t, "base") - defer ht.Finish() - c := NewTestConfig() - c.RateQuota = &throttled.RateQuota{ - MaxRate: throttled.PerHour(10), - MaxBurst: 9, - } - c.RedisURL = "redis://127.0.0.1:6379/" - app := NewApp(c) - defer app.Close() - rh := NewRequestHelper(app) - - redis := app.redis.Get() - _, err := redis.Do("FLUSHDB") - assert.Nil(t, err) - - for i := 0; i < 10; i++ { - w := rh.Get("/") - assert.Equal(t, 200, w.Code) - } - - w := rh.Get("/") - assert.Equal(t, 429, w.Code) - - w = rh.Get("/", test.RequestHelperRemoteAddr("127.0.0.2")) - assert.Equal(t, 200, w.Code) -} - func TestStateMiddleware(t *testing.T) { tt := test.Start(t) defer tt.Finish() From 5974a25d94121474d61914b590b38b8cb0ae497d Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 24 Mar 2020 19:35:46 +0100 Subject: [PATCH 2/2] Reinstate Redis flags and mark as deprecated We should deprecate them first, to give room to remove them (in case someone is using them) --- services/horizon/cmd/root.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/horizon/cmd/root.go b/services/horizon/cmd/root.go index 302cd5fde9..691939c93d 100644 --- a/services/horizon/cmd/root.go +++ b/services/horizon/cmd/root.go @@ -218,6 +218,16 @@ var configOpts = support.ConfigOptions{ }, Usage: "max count of requests allowed in a one hour period, by remote ip address", }, + &support.ConfigOption{ // To be removed in Horizon 2.0.0 + Name: "rate-limit-redis-key", + OptType: types.String, + Usage: "deprecated, do not use", + }, + &support.ConfigOption{ // To be removed in horizon 2.0.0 + Name: "redis-url", + OptType: types.String, + Usage: "deprecated, do not use", + }, &support.ConfigOption{ Name: "friendbot-url", ConfigKey: &config.FriendbotURL,