Commit 496247e
committed
simplify SaramaConsumerGroup.Start method
I believe there was confusion about how this is called or should run. These
changes provide equivalent functionality, while being easier to read and
understand.
There are instructions from the library authors here:
https://github.com/IBM/sarama/blob/v1.38.1/consumer_group.go#L19
I think there was confusion around the steps detailed in the link above. So
let me clarify my understanding:
- Canceling the context passed to Consume is how the implementor of the
sarama.ConsumerGroupHandler tells Consume to return. In other words, when
we're shutting down, we cancel the context to get Consume to return. I base
this off of #4 in the link.
- No goroutine needs to be launched on our side, the sarama library will
launch the goroutines and those goroutines will call our implementation of
ConsumeClaim(). This comes from #3.
- The warnings about thread-safety apply to our implementation of
ConsumeClaim(). In other words, we need to be careful that s.consumer isn't
nil, but since nothing in our code modifies s.consumer after instantiation,
we're fine. This comes from #3.
- Since our own events.EventConsumer has to be able to respond to Stop()
calls, we need to be able to break the for-loop in Start(), which is done by
canceling the context that we pass to Consume(). The context.CancelFunc is
guarded with a mutex for for extra thread-safety, so that if Stop() is called
from multiple goroutines, we don't have any race conditions to worry about.
We can reason that the goroutine being previously launched was not necessary,
because there was only a single goroutine being launched, and the same
function that launched the goroutine then blocked, waiting for it to exit
before continuing.
Without the goroutine, the WaitGroup, stop channel and stopOnce are no longer
necessary.1 parent 2b62b02 commit 496247e
1 file changed
+32
-59
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
6 | 5 | | |
7 | 6 | | |
8 | 7 | | |
| |||
17 | 16 | | |
18 | 17 | | |
19 | 18 | | |
20 | | - | |
21 | | - | |
22 | 19 | | |
23 | | - | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
24 | 23 | | |
25 | 24 | | |
26 | 25 | | |
| |||
32 | 31 | | |
33 | 32 | | |
34 | 33 | | |
35 | | - | |
36 | | - | |
37 | | - | |
38 | 34 | | |
39 | 35 | | |
40 | 36 | | |
| |||
70 | 66 | | |
71 | 67 | | |
72 | 68 | | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
95 | | - | |
96 | | - | |
97 | | - | |
98 | | - | |
99 | | - | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
100 | 80 | | |
| 81 | + | |
101 | 82 | | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | 83 | | |
115 | | - | |
116 | | - | |
117 | 84 | | |
118 | 85 | | |
119 | 86 | | |
120 | | - | |
121 | | - | |
122 | | - | |
123 | | - | |
| 87 | + | |
| 88 | + | |
124 | 89 | | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | | - | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
129 | 96 | | |
130 | | - | |
131 | | - | |
132 | | - | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
133 | 106 | | |
134 | 107 | | |
135 | 108 | | |
| |||
0 commit comments