Action Items

This repo has been audited as a production component. Please respond in 4 weeks.

This repo was found missing required branch protections:

{
  "dismiss_stale_pr_reviews": {
    "error_message": "__required_pull_request_reviews.dismiss_stale_reviews__ Expected dismissal of stale PR approvals, found false.",
    "expected": true,
    "actual": false
  },
  "enforce_admins": {
    "error_message": "__enforce_admins__ Admins should not be allowed to bypass protections.",
    "expected": true,
    "actual": false
  }
}

If this audit is incorrect, or this repo does not need the outlined branch protections please file for an exception here.

• NOTE, dismiss_stale_pr_reviews is not yet mandatory for this repository, but will be in 4 weeks.

What is this, and why am I just now finding out about this?

See this engineering discussion for more information.

The purpose of this service is to provide with an answer "is it OK to write to the database at this time?".

At this time the service is targeted at MySQL clusters, and bases its response on replication lag. E.g. if all replicas are lagging < 800ms, the answer may be "yes, go ahead and write".

To achieve this functionality the service needs to be aware of the replication topologies, or at least of the relevant replicas. We don't need to consult every single replica in a cluster. Not all are created equal.

Orchestrator

This type of information falls conveniently within orchestrator's jurisdiction. orchestrator is familiar with the replication topologies, has knowledge about the pools, can (and does) access replicas to check their lag, provides command line, API & web based interfaces.

This Issue discusses reasons for [not] choosing to implement the functionality of this service directly within orchestrator

Orchestrator: pro

• orchestrator already knows the topologies. If this throttler service is to eb standalone, it would likely talk to orchestrator or similar to get the list of relevant replicas. It would certainly not do discovery.
• orchestrator already configured with credentials, type of query to ask, has the code to query MySQL servers. A standalone service would copy+paste some existing functionality from orchestrator.
• orchestrator already implements a leadership algorithm.
• Meta reason: doing this via orchestrator means more developers getting to know orchestrator.
• Meta reason: existing community users of orchestrator would get this functionality "for free".
• Meta reason: (internal) overhead of yet-another-setup
• Irrelevant meta: if we run this within orchestrator we don't need to come up with a new name

Orchestrator: why not

• orchestrator persists data ; this service does not need to, and persisting would be a painful overhead.
• orchestrator polls servers once every X seconds; this service would poll many times per second
• as result of the above two bullets, running this within orchestrator would mean a change in paradigm, and rather "a different beast" within orchestrator
• the type of communication between nodes of this service is different than the type of communication between orchestrator nodes, at least until orchestrator supports group communication. orchestrator uses (at this time) persistence to share data. Our service would not want to use persistence (at least almost wouldn't want to, I can think of a couple things that would be persisted).
• Half-meta: at some time in the future this service may apply to other datastores than MySQL, and based on other metrics than replicaiton-lag. Binding it to orchestrator would bind it as a MySQL-specific solution.
• Meta: if this goes open-source, many users would benefit from such service. But running orchestrator just for the sake of throttling would be an overkill. I suspect if this were done via orchestrator people would defer from downloading/running this service
• Meta: if we choose to develop this as a standalone service, we can rather easily, later on, integrate it into orchestrator. The reverse is not true. It would be difficult to extract it from orchestrator.

Current thoughts

My gut feeling says to develop this as standalone. And then we'll see.
It's a small enough project that this should ship quickly.

cc @github/platform-data @github/database-infrastructure @skottler @carlosmn

Action Items

This repo has been audited as a production component. Please respond in 4 weeks.

A corresponding Service Catalog entry was not found. Please add an ownership file to this repo guide. If this audit is incorrect, or this repo does not need to be in the Service Catalog, please file for an exception here.

What is this, and why am I just now finding out about this?

See this engineering discussion for more information.

Say we're measuring replication lag and we have 10 replicas. For some apps, it would be OK if one lags. Maybe two. And it would be better let them lag and have ongoing operations, as opposed to stalling everything.

The suggestion is to have a per-cluster config that indicates how many hosts can be down. This would either be an absolute number, or a ratio/percentile. For smaller setups it makes more sense to have an absolute number (e.g. "1 host can be lagging"). For larger setups it may be better to work by percentile ("up to 5% of hosts may be lagging").

I'm unsure whether to support both.

There is an implicit assumption that a host cannot participate in more than one pool.
When it does, and has different metrics query, the two (or more) query results keep on tripping over each other. While collected correctly, freno only stores the result in a hostname(-port)-context.

So completely different metrics (e.g. one is replication-lag, the other is history-list-length) can both be stored in the same metric.

freno should store the metrics in cluster-hostname(-port) context.

Avoiding a scenario where one would throttle an app and then forget about it. While throttled apps should be visible, our preferred way of disabling apps is by having TTL, and hopefully a reason and a owner as well (this is also similar to orchestrator's begin-downtime and begin-maintenance).

We can agree on a default TTL. We can agree on a default user/reason.

This will need to survive raft snapshot, this serialized/deserialized. Note on deserialization that we can't just reapply TTL; instead we need to computer time remaining from original TTL.

cc @ross who suggested this.

👋 Greetings from @github/devcon !

As you may not know from our many Team posts, we are amidst a migration from Jenkins-based infrastructure to using Build Pipelines for internal CI.

This repository was marked for exemption in the inital Q5 migration. In Q1, however, we intend to fully decommission all of Jenkins. That means the outstanding jobs need to either be deleted or migrated by September 1st.

Our records indicate the following CI jobs are still running on Jenkins for this repository:

• freno-build-deploy-tarball

Please use this as a tracking issue for remediation efforts. We will comment on this issue with reminders as we get closer to decomission date.

Team Assigned This Migration:

@github/devcon

The assignee of this issue is inferred based on a combination of CI Ownership Spreadsheet and Exemption Request. If this is assigned incorrectly, please find the proper owner.

If you have any questions, find us in #build on Slack or File an Issue

Similarly to #56, we still see throttle errors on clusters where throttling shouldn't take place.

The case at hand: cluster impacted is beign throttled when no MySQL replication lag is seen. Looking at the error log, at the time impacted is throttled, a roster update is requested from HAProxy, where parsing of a couple other clusters, unrelated1 and unrelated2 fail.

Failure to parse roster for unrelated1 and unrelated2 should gracefully exit for those two clusters, and should absolutely not impact unrelated.

throttler is a great word, but if this should go open source, then there's a dozen or more throttler named repos.

This service observes replication lag on MySQL servers (maybe someday on other datastores?) and serves "may I write now" questions from apps.

Listing below some ideas for names. Please 👍 / 👎

The choice of a name is surprisingly important at this time, since there's going to be some internal infrastructure (puppet, deployments, CI) that will suffer from a later name change.

Things we'd like to measure:

• am I running?
• am I the leader?
• what's the lag on cluster C, for popular clusters
• count requests from app A
• count approved requests for app A (A requested writes, we said "yes")
• count rejected requests for app A (A requested writes, we said "no")
• others?

We can easily expose metrics via expvars.

We currently use DataDog for monitoring. Should we push metrics to DataDog?

• if so, this is useful: https://www.datadoghq.com/blog/instrument-go-apps-expvar-datadog/
• we would have a config variable indication location of DataDog + API key. If this config is not present, we don't push to datadog.

Hi Team

I am working on deploying the Freno service in k8s using stateful sets with 3 replicas (freno-0, freno-1, freno-2).

I tried to use dns names in RaftBind, RaftNodes but raft consensus doesn’t seem to work. Following is the configuration I have used.

"ListenPort": 9777,
"RaftBind": "freno-1.freno-raft-service.n1stack.svc.cluster.local",
"RaftDataDir": "/var/lib/freno",
"DefaultRaftPort": 9888,
"RaftNodes": [
"freno-0.freno-raft-service. n1stack.svc.cluster.local",
"freno-1.freno-raft-service. n1stack.svc.cluster.local",
"freno-2.freno-raft-service. n1stack.svc.cluster.local"
],

Communication issue in Freno POD Logs:

2021/12/22 17:36:58 [WARN] raft: Clearing log suffix from 296904 to 296904 │
│ 2021/12/22 17:36:59 [WARN] raft: Rejecting vote request from 172.34.179.140:9888 since we have a leader: 172.16.213.139:9888 │
│ 2021/12/22 17:36:59 [WARN] raft: Rejecting vote request from 172.34.179.140:9888 since we have a leader: 172.16.213.139:9888 │
│ 2021/12/22 17:36:59 [DEBUG] raft-net: 172.34.154.61:9888 accepted connection from: 172.34.251.16:34644 │
│ 2021/12/22 17:36:59 [DEBUG] raft-net: 172.34.154.61:9888 accepted connection from: 172.34.251.16:34648 │
│ 2021/12/22 17:36:59 [INFO] raft: Duplicate RequestVote for same term: 332362 │
│ 2021/12/22 17:36:59 [WARN] raft: Duplicate RequestVote from candidate: 172.34.251.16:9888 │
│ 2021/12/22 17:36:59 [WARN] raft: Clearing log suffix from 296905 to 296905 │
│ 2021-12-22 17:37:00 DEBUG raft leader is 172.34.251.16:9888; state: Follower │
│ 2021/12/22 17:37:01 [DEBUG] raft-net: 172.34.154.61:9888 accepted connection from: 172.34.251.16:34856 │
│ 2021/12/22 17:37:01 [DEBUG] raft-net: 172.34.154.61:9888 accepted connection from: 172.34.251.16:34860 │
│ 2021/12/22 17:37:01 [INFO] raft: Duplicate RequestVote for same term: 332363 │
│ 2021/12/22 17:37:01 [WARN] raft: Duplicate RequestVote from candidate: 172.34.251.16:9888 │
│ 2021/12/22 17:37:01 [WARN] raft: Clearing log suffix from 296906 to 296906 │
│ 2021/12/22 17:37:02 [WARN] raft: Heartbeat timeout from "172.34.251.16:9888" reached, starting election │
│ 2021/12/22 17:37:02 [INFO] raft: Node at 172.34.154.61:9888 [Candidate] entering Candidate state │
│ 2021/12/22 17:37:02 [DEBUG] raft: Votes needed: 3 │
│ 2021/12/22 17:37:02 [INFO] raft: Duplicate RequestVote for same term: 332364 │
│ 2021/12/22 17:37:02 [WARN] raft: Duplicate RequestVote from candidate: 172.34.154.61:9888 │
│ 2021/12/22 17:37:02 [DEBUG] raft: Vote granted from 172.34.154.61:9888. Tally: 1 │
│ 2021/12/22 17:37:02 [WARN] raft: Remote peer freno-0.freno-raft-service.n1stack.svc.cluster.local:9888 does not have local node 172.16.215.220:9888 as a peer │
│ 2021/12/22 17:37:02 [DEBUG] raft: Vote granted from freno-0.freno-raft-service.n1stack.svc.cluster.local:9888. Tally: 2 │
│ 2021/12/22 17:37:02 [DEBUG] raft: Vote granted from freno-2.freno-raft-service.n1stack.svc.cluster.local:9888. Tally: 3 │
│ 2021/12/22 17:37:02 [INFO] raft: Election won. Tally: 3 │
│ 2021/12/22 17:37:02 [INFO] raft: Node at 172.34.154.61:9888 [Leader] entering Leader state

It is going in a loop and the leader is changing each and every minute.

If I try with the IPs approach it is working fine. Following is the working configuration.

"ListenPort": 9777,
"RaftBind": "172.34.179.140",
"RaftDataDir": "/var/lib/freno",
"DefaultRaftPort": 9888,
"RaftNodes": ["172.34.179.140", "172.34.251.16", "172.34.154.61"],

The raft consensus and freno service are working as expected with the above IP approach.

We would like to go with the DNS approach with stateful sets as Ips are ever-changing in k8s.
Could you please assist us in what we missing with the DNS approach because of which RAFT communication is not happening as expected?

Thanks in advance.

It would come very handy for testing and deployment.

To have more visibility into freno and state of backend stores we need to export via expvar:

• the aggregated metrics (per store metrics)
• check result on a more granular basis: today the check result is reported per app, but need to also drill down: per-app-per-store, i.e. indicate the exact request.

Hi folks
In the raft node list, it is mentioned that the list is static. How to dynamically add IP addresses to the raft node list .

https://github.com/github/freno/blob/master/build.sh#L75

The check to get a recent version of go fails because 1.10 is seen as 1.1

Various initial TODOs

• name
• initial directory structure
• CI/deployment (GitHub internal)
• config #10
• run as a service
• HTTP, /lb-check #3
• Raft, leader
• backend polling ; concurrency
• metrics
• system tests

We want freno to support MySQL encrypted connections for both:

• Topology (probed) servers
• Backend server

We already support TLS in both orchestrator and gh-ost so we may as well copy some code from there.

Starting with the most basic usage, this Issue will present would-be API endpoints.

• /write-request/<app-name>/<store-type>/<store-name>
  This is the primary request freno would serve: an app connecting and asking "is it OK to write?".
  examples:

  • /write-request/spokes/mysql/maincluster
  • /write-request/gh-ost/mysql/othercluster

  Initially we will only support the mysql store-type. Other stored may be added in the future.

Further comments to this issue will present other API endpoints

It should be in freno's power to skip/ignore metrics from a given host. As example, freno would have a /skip-host/<hostname> API endpoint. This would override any configuration-based listing of servers as well as dynamic HAProxy-based listing.

The operation would be undone with, say /recover-host/<hostname> or similar.

Also, freno should export the list of skipped/ignore hosts, e.g. via /skipped-hosts

Hey @shlomi-noach! After chatting with @samlambert last week, I'm looking a bit at Freno and was hoping you'd answer a couple of questions about using Freno between regions.

Currently we have a mechanism inside Rails that does essentially the same as the sample Freno configs: cheecks replication lag through pt-heartbeat and running InnoDB threads—throttling itself if they exceed a threshold. We cache these results aggressively in Memcached (similar to #51) and the client. I should note that our primary source of lag are migrations in Rails, i.e. large tasks that iterate over a large amount of records to update them ('maintenance tasks') as well as schema changes (LHM, presently, but looking at gh-ost).

However, recently we've started seeing more cases where other regions are lagging more than the latency to the region with the writer. This can be due to a variety of factors, but I'm unsure that our present assumption of: if the slave in the current region can catch up, so can the slave in the other region. This can be due to a variety of factors, difference in hardware, hiccups, etc., but with our increasing number of shards I believe we'll have to accept throttling across regions.

Doing this inside Rails with cross-region connections is sketchy so say the least, which caused me to look at Freno. However, the way I'm reading the documentation, it would require the Freno master in a single region to connect to all other regions' MySQLs. Is that correct, and is this something you do? It would also mean running multiple Freno clusters unless we'd go cross-dc for Freno, when running active:active (as we are, presently). Service discovery is often scoped to a datacenter, so this can get a bit awkward. When first looking at Freno, I was hoping that the agents would be able to run in multiple data-centers with Raft being used for storing the data, rather than a single leader with the data in memory.

Just as an aside, are you seeing Freno long-term turn into a kind of Doorman? We've been searching for something like this for a while to set global throttling rates for accessing various data-stores. Doorman has some really nifty features, would be curious what you think. Would be happy to chat in Slack about this too.

Cheers!

Right now we can configure cluster hostnames via:

• HAProxySettings
• StaticHostsSettings

Two additional low hanging fruit settings can be:

• via URL: expect plaintext listing of hostnames given a specific URL
• via orchestrator: provide URI for orchestrator such as https://my.orchestrator.com/api/cluster/alias/mycluster -- freno will parse the JSON response correctly.
  • also via cluster-pool-instances/:clusterName/:pool (should also support alias)

Specific apps may require different thresholds. These must be equal or smaller than store thresholds (store threshold are the maximum allowed threashold in a system).

Lower thresholds may impose poor man's prioritization: an app that has a lower threshold will less likely succeed to make progress on busy time, while other apps make it through.

Explicit thresholds can take place in static config, and potentially be applied dynamically.