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

feat(db) upstreams and targets to new dao (replaces #3622 and #3660) #3689

Merged
merged 8 commits into from
Aug 10, 2018

Conversation

bungle
Copy link
Member

@bungle bungle commented Aug 10, 2018

The new admin API is fairly similar to the previously existing one, but there are some breaking changes:

  • Lists (GET /upstreams or GET /upstreams/{upstream}/targets) don't accept query-attributes for filtering any more
  • Pagination: the total attribute is gone
  • Format of error messages has changed to new DAO errors.
  • PUT actions now are applied to entities instead of to collections. (PUT /upstreams/foo instead of PUT /upstreams
  • Some error codes have changed:
    • Attempting to use an invalid name name (such an IP address) when updating an upstream has become a 500 error instead of a 400.
    • PUT actions return a 200 on creation instead of 201.
  • timestamps (like created_at) can be presented and stored either with seconds or milliseconds resolution

New features:

  • There is a attribute called next on paginated admin API results.
  • When listing the targets for an upstream with db.targets:for_upstream(u), the list only includes active targets, and is reverse-sorted by created_at+id. This is the method used in /upstreams/{upstream}/targets
  • There is also a db.targets:for_upstreams_raw(u) which returns all targets, including those with weight=0, associated with the upstream. That's what used in /upstreams/{upstream}/targets/all
  • db.targets:for_upstream_with_health(u) is the same list as db.targets:for_upstream(u) but also includes health information (so it is slower to call). It is used in /upstreams/{upstream}/health.
  • In addition to those there's db.targets:for_upstream_sorted(u) which returns the targets sorted by created_at+id. It is used internally in a couple places for things like cleaning up and building the cache.
  • The db.targets:delete method creates a new target with weight = 0. The only way to really delete a target from the database is by inserting targets (this will eventually trigger a clean_history call).
  • The the db.targets:post_health is the one-place for changing health status of a target. It deals with balancer changes and sending events to the cluster. It is used in POST /upstreams/{upstream}/targets/{target}/healthy|unhealthy.

Prerequisites:

  • Cassandra migrations where not re-entrant (they can not "catch exceptions and continue" like postgres can). In the past the tests where run only in postgresql. I had to add a way to "ignore errors" in order to allow re-entrant migrations in Cassandra.
  • There was a lurking bug related with partial updates of record-type fields.
  • Needed a new kind of validator to allow validating the hash_on_header is not equal to hash_fallback_header
  • The target.target attribute is an endpoint key but it is not unique. Had to remove that restriction from the system.
  • Timestamps should have option for millisecond-precision, but until we updated the targets to the new DAO we didn't see need for that (it is added as an option, and targets entity uses milliseconds precision)

This PR replaces #3622 and #3660.

@bungle bungle changed the title (feat) upstreams and targets to new dao (replaces #3622) (feat) upstreams and targets to new dao (replaces #3622 and #3660) Aug 10, 2018
@bungle bungle requested a review from hishamhm August 10, 2018 15:53
@bungle bungle force-pushed the feat/targets-to-new-dao-rebased branch from 644cd01 to ef5b630 Compare August 10, 2018 16:02
@@ -270,20 +264,24 @@ do
healthcheckers[balancer] = hc

balancer.report_http_status = function(ip, port, status)
log(DEBUG, "HC ", tostring(hc), " FOR BALANCER ", tostring(balancer), " REPORTING HTTP STATUS ", ip, ":", port, " ", status)
Copy link
Contributor

Choose a reason for hiding this comment

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

all these lines containing uppercase DEBUG logs (plus the print below) should be removed

@bungle bungle force-pushed the feat/targets-to-new-dao-rebased branch from ef5b630 to 7a7bbf5 Compare August 10, 2018 17:49
@@ -12,6 +12,7 @@ local log = ngx.log
local sleep = ngx.sleep
local min = math.min
local max = math.max
local inspect = require("inspect")
Copy link
Contributor

Choose a reason for hiding this comment

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

this is also a leftover

@bungle bungle force-pushed the feat/targets-to-new-dao-rebased branch from 7a7bbf5 to 2eebcc6 Compare August 10, 2018 17:53
@hishamhm hishamhm changed the title (feat) upstreams and targets to new dao (replaces #3622 and #3660) feat(db) upstreams and targets to new dao (replaces #3622 and #3660) Aug 10, 2018
@bungle bungle merged commit b92b7bc into next Aug 10, 2018
@bungle bungle deleted the feat/targets-to-new-dao-rebased branch August 10, 2018 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants