KAFKA-16985: Ensure consumer attempts to send leave request on close even if interrupted by kirktrue · Pull Request #16686 · apache/kafka

kirktrue · 2024-07-24T23:17:07Z

This change ensures that AsyncKafkaConsumer attempts to leave the group cleanly upon close(), regardless of the timeout or interrupts. The application thread will invoke the ConsumerRebalanceListener’s callback, if present, to release the assignment, and then enqueue a LeaveGroupOnClose for the background thread. The background thread will send out the “leave group” heartbeat, and will notify the application thread (if still waiting) upon response.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

…errupted The consumer should attempt to leave the group cleanly upon close(), regardless of a) the timeout, b) interrupts. If the user interrupted the current thread, upon close(), note the interruption, clear the flag, proceed with the close logic, and then throw an InterruptException at the end of close(). If the user invokes close() with a low timeout, make sure to process the background events even if the UnsubscribeEvent itself timed out. Those background events are needed to signal the 'leave group' action in the background thread.

…umer group

…is flaky

kirktrue · 2024-08-01T17:21:46Z

@lianetm @philipnee—can I get a review, please 😄

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

kirktrue · 2024-08-01T23:29:00Z

Latest test failures look unrelated.

core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala

…oth consumer types

…f needed

… the application thread

lianetm

Thanks for the updates @kirktrue ! Here are some comments after another pass to non-testing files

lianetm · 2024-11-05T20:15:01Z

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

+        SortedSet<TopicPartition> droppedPartitions = new TreeSet<>(TOPIC_PARTITION_COMPARATOR);
+        droppedPartitions.addAll(assignedPartitions);
+
+        boolean isThreadInterrupted = Thread.currentThread().isInterrupted();


this is not needed anymore right?

This was removed already. Not sure how it was still showing for you 🤔

This was the review I mentioned I started adding comments one day, and finished the day after he he, that's probably why.

lianetm · 2024-11-05T20:18:55Z

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

        applicationEventHandler.add(new CommitOnCloseEvent());
+    }
+
+    private void releaseAssignmentOnClose(final Timer timer) {


nit: matter of changed perception I guess, but what about renaming this to clearly indicate that it is running callbacks? ( ~runRebalanceCallbacksOnClose). I see it's important given the challenges that callbacks bring here

Renamed to runRebalanceCallbacksOnClose() as suggested 👍

lianetm · 2024-11-05T20:38:34Z

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

+        ConsumerGroupMetadata cgm = groupMetadata.get().orElse(null);
+
+        if (cgm == null)
+            return;


up to you, but what about:

Suggested change

ConsumerGroupMetadata cgm = groupMetadata.get().orElse(null);

if (cgm == null)

return;

if (!groupMetadata.get().isPresent())

return;

int memberEpoch = groupMetadata.get().get().generationId();

I find it's comes back clearer later on, to refer to member epoch instead of generation id to determine the callback

Refactored as suggested 👍

lianetm · 2024-11-05T21:50:21Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

+     * @return Future that will complete when the callback execution completes and the heartbeat
+     * to leave the group has been sent out.
+     */
+    protected CompletableFuture<Void> leaveGroup(Supplier<CompletableFuture<Void>> callbackFutureSupplier) {


why passing a Supplier of CompletableFuture instead of simply passing the CompletableFuture itself? seems more verbose/obfuscated but I could be missing an upside.

In any case passing an Optional maybe suits better? given that we want to represent that the leave may (or may not) receive a future to wait on before sending the request

It's an ordering issue 😢

To pass the CompletableFuture into the (internal) leaveGroup() method, the code would have to execute signalMemberLeavingGroup() first. For the ConsumerMembershipManager, signalMemberLeavingGroup() is overridden and calls invokeOnPartitionsRevokedOrLostToReleaseAssignment(). I assumed that invoking invokeOnPartitionsRevokedOrLostToReleaseAssignment() before calling leaveGroup() would throw off the existing logic.

That said, I don't like the use of Supplier here, either. I also wanted to leave the existing code as is, where possible.

Let me know if you have ideas to simplify it.

lianetm · 2024-11-06T20:44:16Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

            if (exception == null) {
                // Enable newly added partitions to start fetching and updating positions for them.
                subscriptions.enablePartitionsAwaitingCallback(addedPartitions);
+                notifyAssignmentChange(addedPartitions);


we should notify with the full assignment here I would say (assignedPartitions)

Done. I should have looked more closely at the actual variable name of addedPartitions 🤦‍♂️

lianetm · 2024-11-06T20:56:05Z

.../main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventProcessor.java

-            if (subscriptions.assignFromUser(new HashSet<>(event.partitions())))
+            if (subscriptions.assignFromUser(new HashSet<>(partitions))) {
                metadata.requestUpdateForNewTopics();
+                requestManagers.consumerMembershipManager.ifPresent(cmm -> cmm.notifyAssignmentChange(new HashSet<>(partitions)));


uhm this is tricky, I wonder if dangerous. Here we are saying that the membership mgr will notify about an assignment that is not really a group assignment (it's manual assign). That notification is then taken on the consumer close to trigger callbacks and send a request to the coordinator (I expect it will all be no-op or throw errors at some point?). I know there is a group check on those operations on close, but we could have groupId but not be in a group if there was never a call to subscribe.

If what we really need in the consumer is a snapshot of group assignment (needed to trigger callbacks), why don't we better keep just that (explicit name, it's not the consumer assignment, it's the consumer group assignment), and do not involve that in this call which is about manual assign? Limiting the scope to what we really need seems to avoid trouble in this case

Good catch. I've made the snapshot targeted only for consumer group members. I'd love for it to be more general, but this is the right call. PTAL at the changes when you have the time. Thanks!

lianetm · 2024-11-06T21:10:25Z

...src/main/java/org/apache/kafka/clients/consumer/internals/events/LeaveGroupOnCloseEvent.java

+ * to perform the necessary steps to leave the consumer group cleanly, if possible. The event's timeout is based on
+ * either the user-provided value to {@link Consumer#close(Duration)} or
+ * {@link ConsumerUtils#DEFAULT_CLOSE_TIMEOUT_MS} if {@link Consumer#close()} was called. The event is considered
+ * complete when the membership manager sends the heartbeat message to leave the group. The event does not wait on a


This event does wait for the response from the coordinator right? It's called with addAndGet, and this event future completes when the leaveGroupOnClose in the membershipMgr completes, and that only happens on maybeCompleteLeaveInProgress, when we receive a response to the HB to leave (or skip HB, or get error).

We intentionally did that to keep the same behaviour of the classic consumer (coordinator does awaitPendingRequests on close), and to avoid responses to disconnected clients that we used to have before, when we would send the leave and carry on with the consumer close/shutdown without waiting for a response. Makes sense?

The intention is that the event should wait until the response has been received for those cases where the timeout is sufficient. I've updated the comment/documentation.

kirktrue · 2024-11-07T20:53:22Z

Thanks for the review, @lianetm 🙏

…r appropriate to invoke externally

lianetm

Hey @kirktrue, thanks for the updates!

lianetm · 2024-11-08T14:16:45Z

.../main/java/org/apache/kafka/clients/consumer/internals/events/ApplicationEventProcessor.java

        requestManagers.commitRequestManager.get().signalClose();
    }

+    private void process(@SuppressWarnings("unused") final LeaveGroupOnCloseEvent event) {


is this suppressWarning needed? (the param is used to complete the future)

lianetm · 2024-11-08T14:35:26Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

+     * @return Future that will complete when the callback execution completes and the heartbeat
+     * to leave the group has been sent out.
+     */
+    protected CompletableFuture<Void> leaveGroup(boolean isClosing) {


would it be clearer to have this param explicitly naming what it does vs. when it's used? (ie. runCallbacks). Mainly because this manager knows nothing about closing really, it knows about leaving a group with or without callbacks)

Renamed to runCallbacks.

lianetm · 2024-11-08T14:51:55Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

        leaveGroupInProgress = Optional.of(leaveResult);

-        CompletableFuture<Void> callbackResult = signalMemberLeavingGroup();
+        CompletableFuture<Void> callbackResult = signalMemberLeavingGroup(isClosing);


I wonder if we could avoid propagating the isClosing/runCallbacks to this func that is really about "invokeCallbacks" (we're effectively calling an "invokeCallbacks" func, passing a param "runCallback" that can be false). This seems confusing and could lead to misleading logs I would say (ie. we would end up with the log line "Member completed callbacks..." when it really didn't do that here).

So one idea that comes to mind is to shortcircuit before the signalMemberLeavingGroup:

if (isClosing/runCallbacks) { CompletableFuture<Void> callbackResult = signalMemberLeavingGroup(); callbackResult.whenComplete((result, error) -> { // log callback completion ... clearAssignmentAndSendLeave(); // encapsulate what we do after callbacks }); } else { clearAssignmentAndSendLeave(); }

What do you think?

Added clearAssignmentAndSendLeave() method as suggested.

lianetm · 2024-11-08T14:54:47Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java


+    /**
+     * Invokes the {@link MemberStateListener#onGroupAssignmentUpdated(Set)} callback for each listener when the
+     * set of assigned partitions changes. This includes on assignment changes, unsubscribing, and when leaving


typo unsubscribing

lianetm · 2024-11-08T14:58:46Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

+                Set<TopicPartition> allAssignedPartitions = assignedPartitions.stream()
+                    .map(tip -> new TopicPartition(tip.topic(), tip.partition()))
+                    .collect(Collectors.toSet());


we can reuse the toTopicPartitionSet already defined in this same class

Done. Thanks, the code was ugly 😅

lianetm · 2024-11-08T15:20:30Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

+                    .map(tip -> new TopicPartition(tip.topic(), tip.partition()))
+                    .collect(Collectors.toSet());
+
+                notifyAssignmentChange(allAssignedPartitions);


uhm this should probably go right after the subscription is updated, not after the callback complete. Having it here, I guess it would allow for a race if there is a call in the app thread to consumer.close, while a reconciliation is running this code. If the call to consumer.close happens after callbacks complete on ln 1167 above, but before making it to this line, the app thread could potentially close revoking partitions without including the ones just added here right?

So we could just call this notify right after updateSubscriptionAwaitingCallback (which is the one that does the actual update of the subscription state). From that moment on the partitions are effectively assigned even though the onPartitionsAssigned may have not completed.

Moved notifyAssignmentChange() to right after subscriptions.assignFromSubscribedAwaitingCallback() in updateSubscriptionAwaitingCallback().

lianetm · 2024-11-08T15:35:39Z

clients/src/test/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumerTest.java

+        Duration timeout = Duration.ofMillis(timeoutMs);
+
+        try {
+            Thread.currentThread().interrupt();


do we need to interrupt if we're already mocking the interrupt exception above? ln 698

Good call. Removed.

lianetm · 2024-11-08T15:37:26Z

clients/src/test/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumerTest.java

-            consumer.subscriptions().assignFromUser(new HashSet<>(event.partitions()));
+            HashSet<TopicPartition> partitions = new HashSet<>(event.partitions());
+            consumer.subscriptions().assignFromUser(partitions);
+            consumer.setGroupAssignmentSnapshot(partitions);


this func if about manual assignment so I would expect we don't need to set the group assignment snapshot right?

You're right. That's a holdover from when I was updating the snapshot for manually-assigned partitions too.

lianetm · 2024-11-08T15:43:29Z

core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala

+
+  @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+  @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+  def testCloseLeavesGroupOnInterrupt(quorum: String, groupProtocol: String): Unit = {


should we add a very similar test for close(0)?

I worked up the test, but I ran it 10 times and it failed twice. The 'leave group on 0 timeout' is a best effort, so the timing may not work out that we can send off the request before the ConsumerNetworkThread shuts down.

ack, makes sense. It's still important to ensure we run callbacks on close(0) but that is covered on testCloseLeavesGroupDespiteOnPartitionsLostError, all good.

lianetm

Thanks for the updates @kirktrue !

Could you please update the PR description to remove all the old callback flow that is not applied on close anymore? Important to call out that this is ensuring callbacks executed + attempt to send the leave request, regardless of timeout/interrupt.

lianetm · 2024-11-11T14:40:34Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

-        Collection<TopicPartition> assignedTopicPartitions = toTopicPartitionSet(assignedPartitions);
+        Set<TopicPartition> assignedTopicPartitions = toTopicPartitionSet(assignedPartitions);
        subscriptions.assignFromSubscribedAwaitingCallback(assignedTopicPartitions, addedPartitions);
+        notifyAssignmentChange(assignedTopicPartitions);


we are notifying on the path of new assignment and on leave, but I think we're missing the fenced/fatal paths.

Those 2 end up calling clearAssignment, so maybe we could notify there to cover them? (right after ln 483 subscriptions.assignFromSubscribed(Collections.emptySet()))?

I changed the code to look like this:

private void clearAssignment() { if (subscriptions.hasAutoAssignedPartitions()) { subscriptions.assignFromSubscribed(Collections.emptySet()); notifyAssignmentChange(Collections.emptySet()); } currentAssignment = LocalAssignment.NONE; clearPendingAssignmentsAndLocalNamesCache(); }

lianetm · 2024-11-11T16:29:57Z

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

+     *         The {@link #close(Duration)} method should otherwise honor the timeout and interrupt flag,
+     *         except where it would violate the previous tenets.


I wonder if we're making it sound more complicated than it is, by saying "except where it would violate the previous tenets" and given that there are 12 tenets above :). Close always honours timeout except for callbacks only right? (already mentioned in point 4). If we simplify as mentioned above, adding to 4 the fact that callbacks are not time-bounded, I would say we don't need to have this one maybe?

Fair enough. Removed.

lianetm · 2024-11-11T16:33:37Z

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

+     *         Leaving the consumer group is achieved by issuing a ‘leave group‘ network request. This network I/O
+     *         must be performed on the background thread.


true but this is not specific to the close in any way right? (It's the same for all api calls that issue requests). Would it be better/simpler to focus on the tenets of the close itself?

I agree, but there was some discussion earlier in this thread which suggested clarification around which thread does what part of the overall task. I removed it, per your suggestion.

lianetm · 2024-11-11T16:40:31Z

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

+     *         Since the {@link ConsumerRebalanceListener} APIs do not include a timeout parameter, a given
+     *         {@link ConsumerRebalanceListener} implementation cannot alter its behavior to adhere to the timeout.


I find this is closely related to the one above (said in a way there already). Callbacks block for the full execution, not time-bounded, that's it. Could we simplify by merging them? (just adding the "not time bounded" bit above). Trying to simplify, this doc got a bit too complicated I would say.

I consolidated these points. LMK if it still needs work.

lianetm · 2024-11-11T17:03:18Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

+    /**
+     * Transition to {@link MemberState#PREPARE_LEAVING} to release the assignment. Once completed,
+     * transition to {@link MemberState#LEAVING} to send the heartbeat request and leave the group.
+     * This is expected to be invoked when the user calls the unsubscribe API.


Suggested change

* This is expected to be invoked when the user calls the unsubscribe API.

* This is expected to be invoked when the user calls the unsubscribe API or is closing the consumer.

I made the change as suggested.

…vs. manually-assigned)

lianetm

Thanks @kirktrue, just these minor comments, almost there.

lianetm · 2024-11-13T13:59:50Z

...nts/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractMembershipManager.java

+    private void clearAssignmentAndLeaveGroup() {
+        subscriptions.unsubscribe();
+        clearAssignment();
+        notifyAssignmentChange(Collections.emptySet());


this is not needed here anymore right? (because clearAssignment will do)

Correct. I removed the extra call since clearAssignment() handles it where appropriate anyway.

lianetm · 2024-11-13T14:01:33Z

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java

+     *     </li>
+     * </ol>
+     */
+


nit: extra line

lianetm · 2024-11-13T14:05:19Z

core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala

+
+  @ParameterizedTest(name = TestInfoUtils.TestWithParameterizedQuorumAndGroupProtocolNames)
+  @MethodSource(Array("getTestQuorumAndGroupProtocolParametersAll"))
+  def testCloseLeavesGroupOnInterrupt(quorum: String, groupProtocol: String): Unit = {


ack, makes sense. It's still important to ensure we run callbacks on close(0) but that is covered on testCloseLeavesGroupDespiteOnPartitionsLostError, all good.

…nment() right above already notifies

lianetm

Thanks a lot @kirktrue! LGTM

kirktrue · 2024-11-13T19:39:13Z

Thank you ♾️ @lianetm and @chia7712!!!! Thanks also to everyone else who helped define and frame the problem.

…even if interrupted (apache#16686) Reviewers: Andrew Schofield <aschofield@confluent.io>, Chia-Ping Tsai <chia7712@gmail.com>, Lianet Magrans <lmagrans@confluent.io>, Philip Nee <pnee@confluent.io>

kirktrue added KIP-848 The Next Generation of the Consumer Rebalance Protocol ctr Consumer Threading Refactor (KIP-848) consumer labels Jul 24, 2024

kirktrue added 13 commits July 26, 2024 15:11

More WiP

6d19c30

Fixed checkstyle errors :(

f52a8b8

Merge branch 'trunk' into KAFKA-16985-clear-interrupt-on-consumer-close

e4b0a27

Attempting to use polling wait for member to join and then leave cons…

6bd52ca

…umer group

Updates to test core issue

980e782

Merge branch 'trunk' into KAFKA-16985-clear-interrupt-on-consumer-close

d113d98

Reverting method name change

abac405

Testing only the new consumer group protocol as the classic consumer …

30f85a6

…is flaky

Reverting name changes

fd563ac

Minor cleanup

ad3c7d9

Update AsyncKafkaConsumer.java

acc3cb4

Update AsyncKafkaConsumer.java

09ccd52

Update AsyncKafkaConsumer.java

c3de041

kirktrue marked this pull request as ready for review August 1, 2024 17:18

kirktrue requested a review from lianetm August 1, 2024 17:19

philipnee reviewed Aug 1, 2024

View reviewed changes

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java Outdated Show resolved Hide resolved

AndrewJSchofield requested changes Aug 1, 2024

View reviewed changes

clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java Outdated Show resolved Hide resolved

kirktrue requested review from AndrewJSchofield and philipnee August 1, 2024 23:29

lianetm reviewed Aug 2, 2024

View reviewed changes

core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala Outdated Show resolved Hide resolved

lianetm reviewed Aug 2, 2024

View reviewed changes

core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala Outdated Show resolved Hide resolved

kirktrue added 2 commits August 2, 2024 17:06

Attempting to generalize testCloseLeavesGroupOnInterrupt for use by b…

a71b251

…oth consumer types

Reverted to only run the integration test only for CONSUMER

3effda7

kirktrue requested a review from lianetm August 21, 2024 23:58

kirktrue added 5 commits November 2, 2024 18:52

Minor test method name tweaks

dda8650

Formatting, comments, refactoring, and restoring the interrupt flag i…

ad4de03

…f needed

Removed efforts to restore interrupt state, post handler

a417e92

Merge branch 'trunk' into KAFKA-16985-clear-interrupt-on-consumer-close

59f128b

Updates to snapshot the assignment in the background thread to use in…

e8b6d99

… the application thread

lianetm reviewed Nov 6, 2024

View reviewed changes

kirktrue added 3 commits November 7, 2024 12:33

Updates to address PR feedback

fa878fe

Merge branch 'trunk' into KAFKA-16985-clear-interrupt-on-consumer-close

af8ef05

Reverting whitespace changes

ac8c7d9

kirktrue added 2 commits November 7, 2024 12:54

Changed the visibility of notifyAssignmentChange since it is no longe…

d635cee

…r appropriate to invoke externally

Addressing the signal-on-close issue from a different approach

ca54979

lianetm reviewed Nov 8, 2024

View reviewed changes

kirktrue added 2 commits November 8, 2024 17:17

Updates to address feedback from PR review

313ae6e

Removed unnecessary import

271638b

lianetm reviewed Nov 11, 2024

View reviewed changes

lianetm mentioned this pull request Nov 11, 2024

KAFKA-17648: AsyncKafkaConsumer#unsubscribe swallow TopicAuthorizationException and GroupAuthorizationException #17516

Merged

3 tasks

kirktrue added 4 commits November 12, 2024 14:54

Notifying when subscription is changed when the assignment is cleared

4b0d310

Updated JavaDoc comments

99d7bc1

Notifying on clearAssignment only if using auto-assigned partitions (…

91a0ec2

…vs. manually-assigned)

Merge branch 'trunk' into KAFKA-16985-clear-interrupt-on-consumer-close

f8d0e1d

lianetm reviewed Nov 13, 2024

View reviewed changes

kirktrue added 2 commits November 13, 2024 06:56

Removed superfluous call to notifyAssignmentChange() since clearAssig…

6dcd3a2

…nment() right above already notifies

Removed extraneous whitespace above close() method

e5d4044

lianetm approved these changes Nov 13, 2024

View reviewed changes

lianetm merged commit b6b2c9e into apache:trunk Nov 13, 2024

kirktrue deleted the KAFKA-16985-clear-interrupt-on-consumer-close branch November 13, 2024 19:39

		* The {@link #close(Duration)} method should otherwise honor the timeout and interrupt flag,
		* except where it would violate the previous tenets.

		* Leaving the consumer group is achieved by issuing a ‘leave group‘ network request. This network I/O
		* must be performed on the background thread.

		* Since the {@link ConsumerRebalanceListener} APIs do not include a timeout parameter, a given
		* {@link ConsumerRebalanceListener} implementation cannot alter its behavior to adhere to the timeout.

	* This is expected to be invoked when the user calls the unsubscribe API.
	* This is expected to be invoked when the user calls the unsubscribe API or is closing the consumer.

Comments

Conversation

kirktrue commented Jul 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

kirktrue commented Aug 1, 2024

Uh oh!

Uh oh!

Uh oh!

kirktrue commented Aug 1, 2024

Uh oh!

Uh oh!

Uh oh!

lianetm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirktrue commented Nov 7, 2024

Uh oh!

lianetm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lianetm Nov 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kirktrue commented Jul 24, 2024 •

edited

Loading

lianetm Nov 8, 2024 •

edited

Loading

lianetm Nov 11, 2024 •

edited

Loading