[FLINK-4280][kafka-connector] Explicit start position configuration for Kafka Consumer #2509

tzulitai · 2016-09-17T14:35:58Z

This is the first PR for a series of new explicit setter configurations for the Kafka Consumer (see also FLINK-3398 for details on opt-out from offset committing).

It adds the following new explicit setter methods to configure the starting position for the Kafka Consumer connector:

FlinkKafkaConsumer08 kafka = new FlinkKafkaConsumer08(...) // or 09
kafka.setStartFromEarliest(); // start from earliest without respecting any committed offsets
kafka.setStartFromLatest(); // start from latest without respecting any committed offsets
kafka.setStartFromGroupOffsets(); // respects committed offsets in ZK / Kafka as starting points

The default is to start from group offsets, so we won't be breaking existing user code.

One thing to note is that this PR also includes some refactoring to consolidate all start offset assigning logic for partitions within the fetcher. For example, in 0.8 version, with this change the SimpleConsumerThread no longer deals with deciding where a partition needs to start from; all partitions should already be assigned starting offsets by the fetcher, and it simply needs to start consuming the partition.This is a pre-preparation for transparent partition discovery for the Kafka consumers in FLINK-4022.

I suggest to review this PR after #2369 to reduce effort in getting the 0.10 Kafka consumer in first. Tests for the new function will be added in follow-up commits.

gyfora · 2016-09-26T14:20:27Z

Hi,

I like the proposed changes, do you think it would make sense to add the possibility to set specific offsets on a per partition basis?

kafka.setStartOffsets(Map<Integer, Long> partitionOffsets)

I think this is extremely useful in production use.

tzulitai · 2016-09-26T15:08:17Z

Hi @gyfora, thanks for the review.
Yes, it is absolutely possible to add that. There's actually a JIRA for that feature too (FLINK-3123), so I'd say we can add that feature on top of the proposed changes here, as a separate follow up PR after this one?

One note though, the API for that feature would need to be able to specify offsets for partitions of different topics, since the Kafka consumers can subscribe multiple topics. So, Map<Integer,Long> wouldn't fit this case, probably would be better off having a new user-facing class as the argument to define the offsets.

tzulitai · 2016-09-26T15:14:17Z

I'll rebase this PR soon, probably will also wait for Kafka 0.10 connector to be merged.

rmetzger · 2016-09-26T17:38:44Z

Thank you for working on this. I gave #2369 some love today to speed up things ;)

gyfora · 2016-09-26T20:36:22Z

@tzulitai makes sense ! As for for the Map<Int, Long> you are right, the multiple topic case slipped my mind :)

koeninger · 2016-10-12T01:47:56Z

...afka-0.8/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumer08.java

-	private static long getInvalidOffsetBehavior(Properties config) {
+	/**
+	 * Check for invalid "auto.offset.reset" values. Should be called in constructor for eager checking before submitting
+	 * the job. Note that 'none' is also considered invalid, as we don't want to deliberately throw an exception


Look out for https://issues.apache.org/jira/browse/KAFKA-3370 if you aren't already aware. Right now allowing none and catching the exception only on startup is the best workaround I've seen.

Thank you for the pointer.
We are discussing this issue here https://issues.apache.org/jira/browse/FLINK-4280 and here https://issues.apache.org/jira/browse/FLINK-3037

Thanks, FLINK-3037 describes what I'm more concerned about, but hadn't seen that ticket yet.

Thanks for pointing this out. To keep things simple for now, I propose to fix https://issues.apache.org/jira/browse/FLINK-3037 as a separate PR.

tzulitai · 2016-10-13T04:10:28Z

Rebasing this now ...

tzulitai · 2016-10-13T08:24:00Z

@rmetzger @gyfora @koeninger Rebased this on the Kafka 0.10 connector and some other recent changes. This is ready for review now ;) I'd like to add tests for this after #2580, because #2580 adds a KafkaOffsetHandler to the Kafka test environment in the IT tests, which will come in handy when writing tests for this PR.

I'll also open a separate PR based on this one for FLINK-3123 (set specific offsets for startup).

tzulitai · 2016-10-13T08:28:31Z

...a-0.9/src/main/java/org/apache/flink/streaming/connectors/kafka/internal/Kafka09Fetcher.java

+							kafkaProperties.getProperty("group.id"), partitionsWithNoOffset);
+						// don't need to do anything; the KafkaConsumer by default finds group offsets from Kafka brokers
+				}
+			} else if (partitionsWithNoOffset.size() > 0 && partitionsWithNoOffset.size() < subscribedPartitions().length) {


@rmetzger after looking at https://issues.apache.org/jira/browse/FLINK-3037, do you think the proposed changes here (in the Kafka09Fetcher, overall) actually fixes that issue?

tzulitai · 2016-10-24T05:40:01Z

Rebased on recent Kafka consumer changes, fixed failing Kafka 0.10 exactly-once tests, and added integration tests (testStartFromEarliestOffsets, testStartFromLatestOffsets, and testStartFromGroupOffsets) for the new explicit startup modes.

However, I'm bumping into kafka ConfigExceptions when instantiating KafkaConsumer in the KafkaOffsetHandler for the testStartFromEarliestOffsets test in versions 0.9 and 0.10. Still investigating the issue, so currently testStartFromEarliestOffsets is deliberately commented out in 0.9 and 0.10 IT tests until I figure out how to fix it, for some early reviews.

rmetzger

Thanks a lot for opening a pull request for this feature. Its something many users requested and many users will like!

Sorry for the big delay with the review.

I have some comments on the proposed changes that need to be addressed before the PR is ready.

rmetzger · 2016-11-22T16:11:37Z

...afka-0.10/src/test/java/org/apache/flink/streaming/connectors/kafka/Kafka010FetcherTest.java

@@ -263,6 +265,7 @@ public Void answer(InvocationOnMock invocation) {
                schema,
                new Properties(),
                0L,
+				StartupMode.GROUP_OFFSETS,


Looks like the indentation of the added lines is correct, but the indentation of the file is wrong. Could you fix that with the PR?

Will do! I think it's actually the other argument lines are indented with spaces instead of tab.

rmetzger · 2016-11-23T08:56:48Z

...tor-kafka-0.10/src/test/java/org/apache/flink/streaming/connectors/kafka/Kafka010ITCase.java

@@ -131,6 +131,25 @@ public void testMetricsAndEndOfStream() throws Exception {
 		runEndOfStreamTest();
 	}

+	// --- startup mode ---
+
+	// TODO not passing due to Kafka Consumer config error


This is easy to fix, right? You just have to put serializer classes into the standardProps.

Hmm, if I recall correctly, that's what I did in the first place, but that caused some other issues. I'll definitely give this another look and make sure the test is runnable.

rmetzger · 2016-11-23T08:57:51Z

...0.10/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaTestEnvironmentImpl.java

-import java.util.List;
-import java.util.Properties;
-import java.util.UUID;
+import java.util.*;


Star imports are something we try to avoid in Flink.

Ah, this was a IDE auto-complete. The style checks don't cover the test codes, right? I'll revert this.

No, we are currently not checking the tests

rmetzger · 2016-11-23T09:01:40Z

...rc/main/java/org/apache/flink/streaming/connectors/kafka/internals/SimpleConsumerThread.java

+		for (KafkaTopicPartitionState<TopicAndPartition> part : partitions) {
+			if (!part.isOffsetDefined()) {
+				throw new RuntimeException("SimpleConsumerThread received a partition with undefined starting offset");


Since we use this in an argument check, we should throw an IllegalArgumentException here.

rmetzger · 2016-11-23T09:03:39Z

...ector-kafka-0.9/src/test/java/org/apache/flink/streaming/connectors/kafka/Kafka09ITCase.java

+//	@Test(timeout = 60000)
+//	public void testStartFromEarliestOffsets() throws Exception {
+//		runStartFromEarliestOffsets();
+//	}


We should fix that as well.

rmetzger · 2016-11-23T09:54:54Z

...-0.8/src/main/java/org/apache/flink/streaming/connectors/kafka/internals/Kafka08Fetcher.java

+							}
+						}
+				}
+			} else if (partitionsWithNoOffset.size() > 0 && partitionsWithNoOffset.size() < subscribedPartitions().length) {


I think this case can currently never happen because on restore, we are only adding partitions part from the restore.

I was adding this as a preparation for the kafka partition discovery task.
But it'd probably make sense to remove it for this PR to avoid confusion.

rmetzger · 2016-11-23T09:57:52Z

...rc/main/java/org/apache/flink/streaming/connectors/kafka/internals/SimpleConsumerThread.java

 			List<KafkaTopicPartitionState<TopicAndPartition>> partitions) throws IOException
 	{
 		// collect which partitions we should fetch offsets for
-		List<KafkaTopicPartitionState<TopicAndPartition>> partitionsToGetOffsetsFor = new ArrayList<>();
+		List<KafkaTopicPartitionState<TopicAndPartition>> partitionsWithEarliestOffsetSetting = new ArrayList<>();


I don't see why you need to copy the partitions into these lists.
I think you can just go over the list and call getLastOffsetFromKafka with part.getOffset() as the last argument.

Good point, didn't think of that. I'll call getLastOffsetFromKafka if getOffset() returns OffsetRequest.EarliestTime() or OffsetRequest.LatestTime().

rmetzger · 2016-11-23T10:01:10Z

...a-0.9/src/main/java/org/apache/flink/streaming/connectors/kafka/internal/Kafka09Fetcher.java

 	protected void emitRecord(T record, KafkaTopicPartitionState<TopicPartition> partition, long offset, ConsumerRecord consumerRecord) throws Exception {
+		// Kafka09Fetcher ignores the timestamp, Kafka010Fetcher is extracting the timestamp and passing it to the emitRecord() method.
 		emitRecord(record, partition, offset, Long.MIN_VALUE);


This will probably break as well when rebasing

rmetzger · 2016-11-23T10:05:19Z

...a-0.9/src/main/java/org/apache/flink/streaming/connectors/kafka/internal/Kafka09Fetcher.java

+					case EARLIEST:
+						LOG.info("Setting starting point as earliest offset for partitions {}", partitionsWithNoOffset);
+
+						seekPartitionsToBeginning(consumer, convertKafkaPartitions(subscribedPartitions()));


Its a bit inefficient to convert the subscribedPartitions() to an ArrayList, and then in seekPartitionsToBeginning the List is converted back into an array. I think we can save the ArrayList step and create an array immediately.

The problem with this one is that the seekToBeginning method broke compatibility from 0.8 to 0.9+.
In 0.8, it's seekToBeginning(TopicPartition...) while in 0.9+ it's seekToBeginning(Collection<TopicPartition>).

I'll integrate these seek methods into the KafkaConsumerCallBridge introduced in a recent PR. It'll be inevitable that we must redundantly do the Array -> List conversion because our subscribedPartitions is an Array, while 0.9+ methods take a collection. For the 0.8 methods, instead of converting the list back to an array, I'll change this to just iterate over the list and call seekPartitionsToBeginning for each one.

rmetzger · 2016-11-23T10:05:26Z

...a-0.9/src/main/java/org/apache/flink/streaming/connectors/kafka/internal/Kafka09Fetcher.java

+					case LATEST:
+						LOG.info("Setting starting point as latest offset for partitions {}", partitionsWithNoOffset);
+
+						seekPartitionsToEnd(consumer, convertKafkaPartitions(subscribedPartitions()));


tzulitai · 2016-11-23T17:32:30Z

Thanks for the review @rmetzger :)
I'll aim to address your comments and rebase by the end of this week (will tag you for another review once it's ready).

tzulitai · 2016-11-28T10:32:46Z

Hi @rmetzger, I've addressed all comments. I'll leave comments inline of code on parts that addresses your more bigger comments, to help with the second-pass review.

tzulitai · 2016-11-28T10:39:51Z

...ka-0.10/src/main/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumer010.java

@@ -128,6 +129,7 @@ public FlinkKafkaConsumer010(List<String> topics, KeyedDeserializationSchema<T>
 	protected AbstractFetcher<T, ?> createFetcher(
 			SourceContext<T> sourceContext,
 			List<KafkaTopicPartition> thisSubtaskPartitions,
+			HashMap<KafkaTopicPartition, Long> restoredSnapshotState,


To make the startup mode logic cleaner, I've changed the AbstractFetcher life cycle a bit.
Now, restored state is provided when constructing the AbstractFetcher, instead of explicitly calling AbstractFetcher#restoreOffsets() as a separate call.

This allows the AbstractFetcher to have a final isRestored flag that version-specific implementations can use once they are created.

The startup offset configuring logic is much simpler now with this flag:

if (isRestored) { // all subscribed partition states should have defined offset // setup the KafkaConsumer client we're using to respect these restored offsets } else { // all subscribed partition states have no defined offset // (1) set offsets depending on whether startup mode is EARLIEST, LATEST, or GROUP_OFFSET // (2) use the fetched offsets from Kafka to set the initial partition states we use in Flink. }

tzulitai · 2016-11-28T10:41:09Z

...tor-kafka-0.10/src/test/java/org/apache/flink/streaming/connectors/kafka/Kafka010ITCase.java

-//		runStartFromEarliestOffsets();
-//	}
+	@Test(timeout = 60000)
+	public void testStartFromEarliestOffsets() throws Exception {


these tests past now with no problem. You're right, setting the key/value deserializer keys did the trick.

tzulitai · 2016-11-28T10:42:40Z

...rc/main/java/org/apache/flink/streaming/connectors/kafka/internals/SimpleConsumerThread.java

 	 *
 	 * @param consumer The consumer connected to lead broker
 	 * @param partitions The list of partitions we need offsets for
 	 * @param whichTime The type of time we are requesting. -1 and -2 are special constants (See OffsetRequest)
 	 */
-	private static void getLastOffsetFromKafka(
+	private static void requestAndSetSpecificTimeOffsetsFromKafka(


Refactored the utility Kafka request methods in this class to avoid creating redundant lists.

tzulitai · 2016-11-28T10:46:49Z

...ka-base/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaConsumerTestBase.java

@@ -482,12 +476,39 @@ public void runStartFromEarliestOffsets() throws Exception {
 	 * ignores the "auto.offset.reset" behaviour as well as any committed group offsets in Kafka.
 	 */
 	public void runStartFromLatestOffsets() throws Exception {


To make this test easier without having to sleep, the test now does this:

First write 50 records to each partition (these shouldn't be read)

Set some offsets in Kafka (should be ignored)

Start a latest-reading consuming job. This jobs throws exception if it reads any of the first 50 records

Wait until the consume job has fully started (added an util method to JobManagerCommunicationUtils for this)

Write 200 extra records to each partition.

Once the writing finishes, cancel the consume job.

Check if the consume job threw any test errors.

tzulitai · 2016-12-08T09:09:06Z

Rebased on the "flink-connectors" change.

rmetzger · 2016-12-08T09:18:17Z

Thanks a lot! I have your Kafka pull requests on my todo list. I hope I get to it soon. I'm really sorry.

tzulitai · 2017-02-04T11:45:46Z

Note about 3rd commit: fixed failing FlinkKafkaConsumerBaseMigrationTests after the rebase.

The tests were failing due to the removal of AbstractFetcher#restoreOffsets(...) method as part of the refactoring of offset restoration in this PR. On the other hand, the previous implementation of tests in FlinkKafkaConsumerBaseMigrationTest were too tightly coupled with how the connector was implemented, i.e. it was testing how the AbstractFetcher methods are called, whether MAX_VALUE watermark was emitted (which will likely change as features are added to the connector) etc, even though the actual purpose of the tests was simply to test states were restored correctly.

The 3rd commit therefore attempts to simplify FlinkKafkaConsumerBaseMigrationTest to only test legacy state restore behaviour. The deleted parts, IMHO, are already covered in other tests.

rmetzger

This is a huge change :) I've quickly scanned the changes again, and I didn't spot anything critical.
I had some questions here and there, but other than that it looks good.

I'll now test the code a bit locally to see if everything behaves as expected, if the logging is done properly etc.
Once I'm done with that and my questions were answered, we can probably merge this change

rmetzger · 2017-02-14T09:27:43Z

docs/dev/connectors/kafka.md

+
+All versions of the Flink Kafka Consumer have the above explicit configuration methods for start position. When
+configured to start from the earliest or latest record by calling either `setStartFromEarliest()` or `setStartFromLatest()`,
+the consumer will ignore any committed group offsets in Kafka when determining the start position for partitions.


Maybe we should add a note that this setting does NOT affect the start position when restoring from a savepoint or checkpoint.

Good point, will add.

rmetzger · 2017-02-14T10:17:57Z

...0.10/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaTestEnvironmentImpl.java

@@ -229,6 +231,8 @@ public void prepare(int numKafkaServers, Properties additionalServerProperties,
 		standardProps.setProperty("zookeeper.connect", zookeeperConnectionString);
 		standardProps.setProperty("bootstrap.servers", brokerConnectionString);
 		standardProps.setProperty("group.id", "flink-tests");
+		standardProps.setProperty("key.deserializer", "org.apache.kafka.common.serialization.ByteArrayDeserializer");
+		standardProps.setProperty("value.deserializer", "org.apache.kafka.common.serialization.ByteArrayDeserializer");


Why is this needed now? If we have this in the default props, we can not ensure that users don't need to set it manually

Somehow, without these settings, the new tests that test setStartFromXXXX methods will fail by complaining the property config does not specify settings for the deserializers.

I guess it is because in those tests, we have Kafka clients that are used only for offset committing and fetching, in which case those clients cannot infer the types to use for the serializers?

Perhaps I should move this out of the standardProps and set them in those tests only.

Yes, move these settings out of the standard properties

Probably its the KafkaOffsetHandlerImpl. In that case, yes, just put the additional properties when creating the instance.

rmetzger · 2017-02-14T10:19:00Z

docs/dev/connectors/kafka.md

+FlinkKafkaConsumer08<String> myConsumer = new FlinkKafkaConsumer08<>(...);
+myConsumer.setStartFromEarliest();     // start from the earliest record possible
+myConsumer.setStartFromLatest();       // start from the latest record
+myConsumer.setStartFromGroupOffsets(); // the default behaviour


Does the "the default behaviour" also mean that we only respect the "auto.offset.reset" configs in that case?

Yes. If the consumer group does not contain offsets for a partition, the "auto.offset.reset" property is used for that partition. I think this is the behaviour of Kafka's high level consumer (and also our connector's original behaviour), so I intended to design it that way.

Okay, cool. Can you add that to the docs as well?

rmetzger · 2017-02-14T14:25:06Z

...ka-base/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaConsumerTestBase.java

+					env.execute(consumeExtraRecordsJobName);
+				} catch (Throwable t) {
+					if (!(t.getCause() instanceof JobCancellationException))
+					error.set(t);


As per the unwritten Flink styleguide, we are always using {} after an if().

Nice catch .. sloppy styling :/

rmetzger · 2017-02-14T14:35:41Z

...st/java/org/apache/flink/streaming/connectors/kafka/FlinkKafkaConsumerBaseMigrationTest.java

-
-		@Override
-		public void close() {
-		}


Looks like you've removed a lot of code from this test here. I guess that the DummyFlinkKafkaConsumer covers everything the deleted code did?

I see. That's what this comment is for: #2509 (comment)

@kl0u I think you've implemented most of the migration tests. Can you take a look at the changes @tzulitai is proposing?

rmetzger · 2017-02-14T16:54:33Z

I've tested the change locally and with great success. So once my comments are addressed, the change is good to be merged

…Consumer

…rebase

tzulitai · 2017-02-15T07:50:44Z

Thanks for the review @rmetzger! The final 2 commits have addressed all your comments.

I'll also wait for @kl0u to have a look at the changes in FlinkKafkaConsumerBaseMigrationTest.
After that and a final run on Travis, I'll merge this to master.

rmetzger · 2017-02-15T09:14:54Z

...ka-base/src/test/java/org/apache/flink/streaming/connectors/kafka/KafkaConsumerTestBase.java

+		Properties offsetHandlerProps = new Properties();
+		offsetHandlerProps.putAll(standardProps);
+		offsetHandlerProps.setProperty("key.deserializer", "org.apache.kafka.common.serialization.ByteArrayDeserializer");
+		offsetHandlerProps.setProperty("value.deserializer", "org.apache.kafka.common.serialization.ByteArrayDeserializer");


Doesn't it make sense to set these properties once in the KafkaOffsetHandlerImpl instead of all locations where the offset handler is being created?

I didn't do that because ideally the constructor of KafkaOffsetHandlerImpl has no knowledge of whether the provided Properties should be manipulated or not (users of KafkaOffsetHandlerImpl provide the properties).

However, I think it would make sense to do what you suggested in the KafkaOffsetHandlerImpl if it always just uses standardProps instead of a provided properties. In our case it would be completely fine to always just use standardProps (plus the deserializer settings), so I'll change it as proposed.

tzulitai · 2017-02-15T10:07:30Z

Last commit addresses consolidating the deserializer settings for KafkaOffsetHandlerImpl.

kl0u · 2017-02-15T15:19:58Z

Hi @tzulitai and @rmetzger . I did not have time so far to look into it. I hope I will be able to do it till the end of the week. Is this ok?

rmetzger · 2017-02-15T15:22:46Z

@tzulitai how does this fit your timeline. Are there PRs depending this or is this PR blocking your in any way?
If so, I would propose that we merge it right away.

tzulitai · 2017-02-15T15:33:44Z

Thanks for letting us know @kl0u!

Yes, there are other pending PRs based on this.
I just double checked the changes in FlinkKafkaConsumerBaseMigrationTest myself, and I think that they are reasonable.

kl0u · 2017-02-15T15:35:39Z

Thanks @tzulitai and @rmetzger ! Of course, feel free to proceed with this.

tzulitai · 2017-02-15T15:38:03Z

Thanks! If do you happen to find inappropriate changes in FlinkKafkaConsumerBaseMigrationTest, please let me know, will be happy to discuss and fix it :-)

Merging this to master now ..

kl0u · 2017-02-15T15:38:46Z

Perfect!

koeninger reviewed Oct 12, 2016

View reviewed changes

tzulitai force-pushed the FLINK-4280 branch from f1d2480 to 80a73d8 Compare October 13, 2016 07:54

tzulitai commented Oct 13, 2016

View reviewed changes

tzulitai force-pushed the FLINK-4280 branch from 80a73d8 to bdf6b76 Compare October 24, 2016 05:25

tzulitai mentioned this pull request Oct 24, 2016

[FLINK-3123] [kafka] Allow custom specific start offsets for Kafka consumers #2687

Closed

rmetzger requested changes Nov 23, 2016

View reviewed changes

tzulitai force-pushed the FLINK-4280 branch from d8f5f97 to 76b1338 Compare November 28, 2016 10:23

tzulitai commented Nov 28, 2016

View reviewed changes

tzulitai force-pushed the FLINK-4280 branch from 76b1338 to eff300e Compare December 8, 2016 08:42

tzulitai force-pushed the FLINK-4280 branch 2 times, most recently from 9a703f8 to 0a84d0a Compare February 4, 2017 11:40

rmetzger reviewed Feb 14, 2017

View reviewed changes

[FLINK-4280] [kafka] Explicit start position configuration for Kafka …

3ef279e

…Consumer

tzulitai added 4 commits February 15, 2017 15:22

[FLINK-4280] Address PR review comments

879c603

[FLINK-4280] Fixed failing FlinkKafkaConsumerBaseMigrationTest after …

d17888e

…rebase

[FLINK-4280] Address final review comments

d73ecfb

[FLINK-4280] Address comments for documentation improvement

2e4b570

tzulitai force-pushed the FLINK-4280 branch from 0a84d0a to 2e4b570 Compare February 15, 2017 07:47

rmetzger reviewed Feb 15, 2017

View reviewed changes

[FLINK-4280] Consolidate deserializer settings for KafkaOffsetHandlers

08dee16

asfgit closed this in 7477c5b Feb 15, 2017

tzulitai mentioned this pull request Feb 21, 2017

[FLINK-5849] [kafka] Move FlinkKafkaConsumer start offset determination to open() #3378

Closed

rmetzger added the component=Connectors/Kafka label Mar 14, 2019

[FLINK-4280][kafka-connector] Explicit start position configuration for Kafka Consumer #2509

[FLINK-4280][kafka-connector] Explicit start position configuration for Kafka Consumer #2509

Conversation

tzulitai commented Sep 17, 2016 • edited Loading

gyfora commented Sep 26, 2016

tzulitai commented Sep 26, 2016 • edited Loading

tzulitai commented Sep 26, 2016

rmetzger commented Sep 26, 2016

gyfora commented Sep 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzulitai commented Oct 13, 2016 • edited Loading

tzulitai commented Oct 13, 2016 • edited Loading

Choose a reason for hiding this comment

tzulitai commented Oct 24, 2016 • edited Loading

rmetzger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzulitai Nov 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzulitai Nov 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzulitai commented Nov 23, 2016 • edited Loading

tzulitai commented Nov 28, 2016

tzulitai Nov 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzulitai commented Dec 8, 2016

rmetzger commented Dec 8, 2016

tzulitai commented Feb 4, 2017

rmetzger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzulitai Feb 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzulitai Feb 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmetzger commented Feb 14, 2017

tzulitai commented Feb 15, 2017 • edited Loading

Choose a reason for hiding this comment

tzulitai Feb 15, 2017 • edited Loading

Choose a reason for hiding this comment

tzulitai commented Feb 15, 2017 • edited Loading

kl0u commented Feb 15, 2017

rmetzger commented Feb 15, 2017

tzulitai commented Feb 15, 2017

kl0u commented Feb 15, 2017

tzulitai commented Feb 15, 2017

kl0u commented Feb 15, 2017

tzulitai commented Sep 17, 2016 •

edited

Loading

tzulitai commented Sep 26, 2016 •

edited

Loading

tzulitai commented Oct 13, 2016 •

edited

Loading

tzulitai commented Oct 13, 2016 •

edited

Loading

tzulitai commented Oct 24, 2016 •

edited

Loading

tzulitai Nov 23, 2016 •

edited

Loading

tzulitai Nov 26, 2016 •

edited

Loading

tzulitai commented Nov 23, 2016 •

edited

Loading

tzulitai Nov 28, 2016 •

edited

Loading

tzulitai Feb 14, 2017 •

edited

Loading

tzulitai Feb 14, 2017 •

edited

Loading

tzulitai commented Feb 15, 2017 •

edited

Loading

tzulitai Feb 15, 2017 •

edited

Loading

tzulitai commented Feb 15, 2017 •

edited

Loading