Refactor hinted-handoff service

[otoolep]

This change refactors the hinted-handoff service.

Whereas previously the system repeatedly launched a short-lived goroutine per active node (to drain all HH data for that node), it now launches a single long-running goroutine per active node which remains running until the node processor is purged due to being marked inactive (which may never happen). A node is considered active until it is DROPped.

Giving each target node a dedicated, long-running, goroutine makes it easier to deal with some HH edge cases.

The change also cleans up stats, and adds HH diagnostics.

> show diagnostics for 'hh'
name: hh
--------
node    active  last modified                           head                            tail
2       yes     2015-10-26T16:33:48.579769662-07:00     /tmp/influxdb8086/hh/2/1:0      /tmp/influxdb8086/hh/2/1:43
3       yes     2015-10-26T16:34:02.055769467-07:00     /tmp/influxdb8086/hh/3/1:0      /tmp/influxdb8086/hh/3/1:2029


> show stats
name: hh
tags: path=/tmp/influxdb8086/hh
wr_shard_req    wr_shard_req_points
------------    -------------------
154             154

name: hh_processor
tags: node=3, path=/tmp/influxdb8086/hh/3
wr_node_req_fail        wr_shard_req    wr_shard_req_points
----------------        ------------    -------------------
1                       154             154

Multiple rounds of testing done with 3 nodes, with the 2 nodes not receiving the write requests killed every 10 seconds, multiple times. After these 2 nodes were finally brought online and kept online, they received all the missed writes.

[pauldix]

target node? Do you mean shard?

[otoolep]

HH queues (which are wrapped in a NodeProcessor) are (and always have been) on a per-node basis. By node I mean an InfluxDB process, server, etc whatever word makes sense.

[pauldix]

Then I don't understand what "target node has been dropped from the server" means. That's "target server has been dropped from the server"? Do you actually mean "the target server has been dropped from the cluster"?

[otoolep]

Yes I do, that Is a typo.

On Friday, October 23, 2015, Paul Dix notifications@github.com wrote:

In etc/config.sample.toml
#4516 (comment):

• enabled = true
• dir = "/var/opt/influxdb/hh"
• max-size = 1073741824
• max-age = "168h"
• retry-rate-limit = 0

• Hinted handoff will start retrying writes to down nodes at a rate of once per second.

• If any error occurs, it will backoff in an exponential manner, until the interval

• reaches retry-max-interval. Once writes to all nodes are successfully completed the

• interval will reset to retry-interval.

• retry-interval = "1s"
• retry-max-interval = "1m"

• Interval between running checks for data that should be purged. Data is purged from

• hinted-handoff queues for two reasons. 1) The data is older than the max age, or

• 2) the target node has been dropped from the server. Data is never dropped until

Then I don't understand what "target node has been dropped from the
server" means. That's "target server has been dropped from the server"? Do
you actually mean "the target server has been dropped from the cluster"?

—
Reply to this email directly or view it on GitHub
https://github.com/influxdb/influxdb/pull/4516/files#r42859470.

[pauldix]

What is a node? I see this referenced a few times, but just a NodeProcessor. Something I'm missing?

[otoolep]

A Node is a distinct InfluxDB server i.e. a node in a cluster.

[otoolep]

@jwilder

[otoolep]

Kicked tests on Circle, it was an unrelated Raft issue.

[jwilder]

Setting this to nil prevents existing goroutines from selecting it. That has created issues in other parts of the code.

[otoolep]

Yeah, I've seen this pattern introduced recently and it made sense to me.

So a nil state of this variable is used to check the "open" state of the service. Sounds like I would then need to use a distinct boolean instead.

[jwilder]

Not sure if the nil channels will be a problem in this code or no. They did cause problems in other places where they were supposed to abort early from a function call, but did not because they were set to nil after being closed. Up to you.

👍

[otoolep]

Use of closing right now should be OK, future enhancements may come.