Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Lock unlock protocol flags #742

Merged

Conversation

eric-sap
Copy link
Contributor

Subtask of #588 to add locks to ConsumerGroupAsyncCommands (dispatcher side)

Proposed Changes
🎁. Added functionality to the managedGroup (resetLockTimer/canUnlock/removeLock) to allow locking and unlocking of a managedGroup via an arbitrary token.
🎁 Changed the stopConsumerGroup and startConsumerGroup functions to call processLock, which understands the fields in the new CommandLock struct of the ConsumerGroupAsyncCommand and will lock/unlock the managedGroup
🧽 Change the Reconfigure function to also obey the managedGroup lock/unlock using an internal CommandLock token

The ResetOffset Controller is still "inactive" and has not been added to any Channels/Sources/etc.
The ResetOffset Controller is still not "complete" and this is a stepping stone in the path to implementing the feature.

Release Note
Release Notes will be provided near the end of the ResetOffset feature development.

Docs
Documentation will be provided near the end of the ResetOffset feature development.

@knative-prow-robot knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 28, 2021
@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 28, 2021
@codecov
Copy link

codecov bot commented Jun 28, 2021

Codecov Report

Merging #742 (f487b46) into main (000dd9c) will increase coverage by 0.34%.
The diff coverage is 78.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #742      +/-   ##
==========================================
+ Coverage   75.29%   75.64%   +0.34%     
==========================================
  Files         132      140       +8     
  Lines        5785     6307     +522     
==========================================
+ Hits         4356     4771     +415     
- Misses       1208     1316     +108     
+ Partials      221      220       -1     
Impacted Files Coverage Δ
...onsolidated/dispatcher/consumer_message_handler.go 0.00% <0.00%> (ø)
.../channel/distributed/controller/util/dispatcher.go 100.00% <ø> (ø)
pkg/source/adapter/adapter.go 58.42% <ø> (-1.08%) ⬇️
pkg/source/client/client.go 15.66% <0.00%> (-19.48%) ⬇️
pkg/source/reconciler/source/kafkasource.go 0.00% <0.00%> (ø)
pkg/source/mtadapter/adapter.go 47.82% <22.72%> (-27.36%) ⬇️
pkg/apis/sources/v1beta1/kafka_lifecycle.go 53.33% <50.00%> (-0.52%) ⬇️
pkg/channel/consolidated/utils/util.go 95.91% <50.00%> (-4.09%) ⬇️
pkg/source/client/offsets.go 60.00% <60.00%> (ø)
pkg/channel/consolidated/dispatcher/dispatcher.go 61.58% <66.66%> (+0.10%) ⬆️
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36d50bf...f487b46. Read the comment docs.

Copy link
Contributor

@matzew matzew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 29, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eric-sap, matzew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit 691c922 into knative-extensions:main Jun 29, 2021
@travis-minke-sap travis-minke-sap mentioned this pull request Jun 29, 2021
13 tasks
@eric-sap eric-sap deleted the lock-unlock-protocol-flags branch October 27, 2021 20:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants