Broker truncation #2276

otoolep · 2015-04-14T02:01:25Z

This change implements basic topic truncation on Brokers. It does this by periodically checking each topic for its size, and if greater than a certain threshold and the topic data has been replicated, deleting segments until the required size is reached.

The change also exposes maximum topic and segment sizes via configuration, as I believe some people may want to change these numbers.

I decided to implement this via a go-routine, so that the function to check the topic sizes could simply walk the filesystem, and not have to worry too much about efficiency vis a vis caching sizes in RAM. I think a design whereby truncation is performed every few minutes is fine (it's how Apache Kafka works), and this approach seems simpler and more robust.

Remaining:

letting data nodes know when they must redirect to other nodes for topic data.

jwilder · 2015-04-14T02:44:34Z

messaging/broker.go

@@ -24,6 +24,12 @@ import (
 // only occurs when the reader is at the end of all the data.
 const DefaultPollInterval = 100 * time.Millisecond

+// DefaultMaxTopicSize is the largest a topic can get before truncation.
+const DefaultMaxTopicSize = 1024 * 1024 * 1024 // 10MB


otoolep · 2015-04-14T03:01:07Z

Thanks @jwilder -- comment was a copy 'n' paste error.

otoolep · 2015-04-14T03:13:30Z

Thanks for feedback @jwilder -- updated.

jwilder · 2015-04-14T04:03:12Z

LGTM. I'm a little concerned about how frequently ReadSegments is called since it's doing a Readdir(0) each time. It's called to get the topic Size() and then to get the segment size in a loop multiple times for each topic. If there are a lot of Topics to truncate, this might cause some sporadic performance issues. Don't think you need to change it now but we might want to see how it performs w/ a large numbers of topics.

otoolep · 2015-04-14T04:47:23Z

@jwilder -- I don't think it would make a huge impact, but it's not great, I agree. Take a look at the modified implementation. I think it's safe, since the topic is locked.

jwilder · 2015-04-14T05:00:20Z

messaging/broker.go

+			return
+		}
+		nBytesDeleted += size
+		segments = segments[1 : len(segments)-1]


I think this is slicing an item off on both ends. Shouldn't it be segments[1:]?

Yeah, it should be. :-) That's what you get when you don't unit-test. :-)

jwilder · 2015-04-14T05:01:10Z

Yeah, that looks better.

otoolep · 2015-04-14T05:24:55Z

OK, updated so there is only a single read of the topic directory. Since the topic is locked, segments can't change since writing to the topic also takes the lock.

Unit test same.

A segment must now have been replicated by at least 1 node before it can be deleted.

otoolep · 2015-04-14T22:54:27Z

messaging/broker.go

@@ -691,6 +744,13 @@ func (t *Topic) IndexForURL(u url.URL) uint64 {
 	return t.indexByURL[u]
 }

+// SetIndexForURL sets the replicated index for a given data URL.
+func (t *Topic) SetIndexForURL(index uint64, u url.URL) {


Exported this so I could easily unit-test.

If a data node requests a topic index that is earier than is present for a topic, tombstones allow the broker to know that the data node should be redirected to another node that has the topic's data already replicated. If no tombstone exists, then the broker can simply restart replaying the topic data it has.

benbjohnson · 2015-04-14T23:03:10Z

messaging/broker.go

+					return
+				}
+			}
+		}()


Can you move this to a named function so it's isolated? Also, the Open() is already kind of big.

Sure, was on the fence about this.

benbjohnson · 2015-04-14T23:08:13Z

Overall lgtm, mostly minor nits.

otoolep · 2015-04-14T23:17:48Z

Thanks @benbjohnson -- maybe I'll just scrap the named returns. Might be clearer.

otoolep · 2015-04-14T23:41:05Z

Named returns scrapped, simpler is better. Green build.

Broker truncation

otoolep added the 2 - Working label Apr 14, 2015

otoolep force-pushed the broker_truncation branch 2 times, most recently from a725955 to dc896ce Compare April 14, 2015 02:02

jwilder reviewed Apr 14, 2015
View reviewed changes

otoolep force-pushed the broker_truncation branch from 30da629 to f8a0925 Compare April 14, 2015 03:01

otoolep force-pushed the broker_truncation branch 3 times, most recently from 207b900 to 13dfdc6 Compare April 14, 2015 03:23

otoolep force-pushed the broker_truncation branch 3 times, most recently from f5f78f9 to 24196e7 Compare April 14, 2015 04:44

otoolep force-pushed the broker_truncation branch 2 times, most recently from 0309647 to 2413ad6 Compare April 14, 2015 04:53

jwilder reviewed Apr 14, 2015
View reviewed changes

otoolep force-pushed the broker_truncation branch from 2413ad6 to 8219f25 Compare April 14, 2015 05:22

otoolep force-pushed the broker_truncation branch 9 times, most recently from d021b6f to b6d68bb Compare April 14, 2015 22:12

otoolep added 5 commits April 14, 2015 15:15

Make MaxTopicSize and MaxSegmentSize configurable

8470ee2

Fix typo in comments

9dbef70

Add broker truncation

b0ee5d0

Unit test same.

Correct out-of-date comment

c9ea5f1

Don't delete unreplicated segments

f6768e5

A segment must now have been replicated by at least 1 node before it can be deleted.

otoolep force-pushed the broker_truncation branch from b6d68bb to e96148c Compare April 14, 2015 22:16

otoolep reviewed Apr 14, 2015
View reviewed changes

otoolep added 2 commits April 14, 2015 15:56

Update CHANGELOG

f591150

otoolep force-pushed the broker_truncation branch from e96148c to f591150 Compare April 14, 2015 22:56

benbjohnson reviewed Apr 14, 2015
View reviewed changes

otoolep force-pushed the broker_truncation branch from b209afd to f6763ef Compare April 14, 2015 23:30

otoolep added 2 commits April 14, 2015 16:35

Code review changes

2002e64

Update sample config with topic truncation

dab100a

otoolep force-pushed the broker_truncation branch from f6763ef to dab100a Compare April 14, 2015 23:35

otoolep added a commit that referenced this pull request Apr 15, 2015

Merge pull request #2276 from influxdb/broker_truncation

ba90815

Broker truncation

otoolep merged commit ba90815 into master Apr 15, 2015

otoolep removed the 2 - Working label Apr 15, 2015

otoolep deleted the broker_truncation branch April 15, 2015 00:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Broker truncation #2276

Broker truncation #2276

otoolep commented Apr 14, 2015

jwilder Apr 14, 2015

otoolep commented Apr 14, 2015

otoolep commented Apr 14, 2015

jwilder commented Apr 14, 2015

otoolep commented Apr 14, 2015

jwilder Apr 14, 2015

otoolep Apr 14, 2015

jwilder commented Apr 14, 2015

otoolep commented Apr 14, 2015

otoolep Apr 14, 2015

benbjohnson Apr 14, 2015

otoolep Apr 14, 2015

benbjohnson commented Apr 14, 2015

otoolep commented Apr 14, 2015

otoolep commented Apr 14, 2015

Broker truncation #2276

Broker truncation #2276

Conversation

otoolep commented Apr 14, 2015

jwilder Apr 14, 2015

Choose a reason for hiding this comment

otoolep commented Apr 14, 2015

otoolep commented Apr 14, 2015

jwilder commented Apr 14, 2015

otoolep commented Apr 14, 2015

jwilder Apr 14, 2015

Choose a reason for hiding this comment

otoolep Apr 14, 2015

Choose a reason for hiding this comment

jwilder commented Apr 14, 2015

otoolep commented Apr 14, 2015

otoolep Apr 14, 2015

Choose a reason for hiding this comment

benbjohnson Apr 14, 2015

Choose a reason for hiding this comment

otoolep Apr 14, 2015

Choose a reason for hiding this comment

benbjohnson commented Apr 14, 2015

otoolep commented Apr 14, 2015

otoolep commented Apr 14, 2015