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.

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

services/horizon: Remove all references to Redis #2409

Merged
merged 2 commits into from
Mar 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion DEVELOPING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion go.list
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
19 changes: 8 additions & 11 deletions services/horizon/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,17 +218,15 @@ 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{ // To be removed in Horizon 2.0.0
Name: "rate-limit-redis-key",
OptType: types.String,
Usage: "deprecated, do not use",
},
&support.ConfigOption{
Name: "redis-url",
ConfigKey: &config.RedisURL,
OptType: types.String,
Usage: "redis to connect with, for rate limiting",
&support.ConfigOption{ // To be removed in horizon 2.0.0
Copy link
Member

Choose a reason for hiding this comment

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

We used to add a specially formatted comment following the pattern outlined in issue #833 so that CI picks up on them and helps us remove them when the time comes. @stellar/horizon-committers I can't find any details in our CONTRIBUTING.md or DEVELOPING.md documents that outline how to do this. Have we abandoned that practice?

We still have the CI check for this (below), so I think we're still wanting to do it and probably need some better docs to encourage us to use it. I know I've probably not done this properly recently too 😬.

go/.circleci/config.yml

Lines 41 to 52 in bab6abc

# check_deprecations ensures a release is actually removing deprecated fields
# that were supposed to be discontinued in said release.
check_deprecations:
steps:
- run:
name: Run deprecation tests when on a tagged commit
command: |
if [ "$CIRCLE_TAG" != "" ]; then
# Negate the result so process exits with 1 if anything found
echo "Searching for \"action needed\" tags..."
! egrep -irn -A 1 --include=*.go "Action.+needed.+in.+release:.+$CIRCLE_TAG" ./
fi

Copy link
Member

Choose a reason for hiding this comment

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

Good catch. The approach isn't abandoned. It's documented in our internal devel guide.

Mark deprecated public API fields using deprecation tag:
// Deprecated - remove in: horizon-v[version]

We should do a pass over this and update DEVELOPING.md with things relevant to all contributors.

@2opremio can you update the deprecation comments to match the syntax quoted above?

Copy link
Member

Choose a reason for hiding this comment

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

#2412 for the doc updates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure thing

Name: "redis-url",
OptType: types.String,
Usage: "deprecated, do not use",
},
&support.ConfigOption{
Name: "friendbot-url",
Expand Down Expand Up @@ -395,7 +393,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.
Expand Down
5 changes: 0 additions & 5 deletions services/horizon/internal/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -40,7 +39,6 @@ type App struct {
coreQ *core.Q
ctx context.Context
cancel func()
redis *redis.Pool
coreVersion string
horizonVersion string
currentProtocolVersion int32
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions services/horizon/internal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions services/horizon/internal/docs/admin.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 1 addition & 7 deletions services/horizon/internal/docs/notes_for_developers.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,7 @@ Check existing tests for more examples.

## <a name="tests"></a> 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 ./...
Expand Down
60 changes: 3 additions & 57 deletions services/horizon/internal/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
33 changes: 0 additions & 33 deletions services/horizon/internal/init_test.go

This file was deleted.

30 changes: 0 additions & 30 deletions services/horizon/internal/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down