-
Notifications
You must be signed in to change notification settings - Fork 237
improve: filter only own updates for read-after-write-consistency #3414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
57 commits
Select commit
Hold shift + click to select a range
3c84b16
improve: filter only own updates for read-after-write-conistency
csviri 9d9571a
wip
csviri 0d7fe34
wip
csviri 50fc62c
wip
csviri 2b76ac2
Event filtering with recording
csviri de1fdb4
test fix
csviri 3ff2f62
Simplified EventHandling
csviri 42087b9
unit tests fix
csviri e206edf
small fix, test repeats
csviri caa75ab
improvements and releated unit tests
csviri 28ee8f3
cleanup
csviri a2548d1
improve: filter only own updates for read-after-write-conistency with…
csviri 29834e1
improvements on edge cases
csviri ae7e49e
Potential fix for pull request finding
csviri 12c84e9
delete related improvements and unit tests
csviri 0b60010
delete handling improvements and test improvements
csviri fa55237
wip
csviri c8bfdcb
tests
csviri f89a3d3
test fix
csviri 53e8876
fix typo
csviri 8c7d4db
Potential fix for pull request finding
csviri 4955b0c
fixes
csviri 7be9fd6
improve: filter only own updates for read-after-write-conistency with…
csviri 0340319
test fixes
csviri e88b7f5
logging and improvements
csviri 39b0ac3
test AI identified cases
csviri 8b2d6be
fix: only filter own events
csviri 2b84952
wip
csviri f529dab
improvements and test fixes
csviri a8a92ab
improvements
csviri 9585d74
fix resource cache read
csviri 6f0e857
support for re-list
csviri d2e406f
simple algorithm, refined tests
csviri f31825a
naming fix
csviri 7cb0a41
small fixes
csviri fc3e984
cleanup
csviri e25d598
cleanup
csviri a3112be
additional tests
csviri ff2cc50
addiotinal tests and docs improvements
csviri a97f314
wip
csviri d3c528c
wip
csviri db033f9
prepare for re-list
csviri f19429d
wip
csviri 880f888
wip
csviri 76e8c83
Potential fix for pull request finding
csviri 842bddd
wip
csviri f084821
fix filtering
csviri d3ed86a
logging
csviri d3fb737
revert trace log
csviri 2e84d7e
addressed issues from code review
csviri 9adbca4
wip
csviri 831be5d
minor improvements
csviri 3f494d4
rename back GenericResourceEvent to ExtendedResourceEvent
csviri 5741aea
fix: typo
metacosm 32eba12
documenting algorithm
csviri 3bfa595
further optimizations
csviri 4fa883b
fix: typo
metacosm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
72 changes: 0 additions & 72 deletions
72
...java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java
This file was deleted.
Oops, something went wrong.
128 changes: 128 additions & 0 deletions
128
...java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupport.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| /* | ||
| * Copyright Java Operator SDK Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.javaoperatorsdk.operator.processing.event.source.informer; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import io.javaoperatorsdk.operator.processing.event.ResourceID; | ||
|
|
||
| class EventFilterSupport { | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(EventFilterSupport.class); | ||
|
|
||
| private final Map<ResourceID, EventFilterWindow> eventFilterWindows = new HashMap<>(); | ||
| private boolean ongoingReList = false; | ||
|
|
||
| public synchronized void startEventFilteringModify(ResourceID resourceID) { | ||
| var existing = eventFilterWindows.get(resourceID); | ||
| var ed = | ||
| eventFilterWindows.computeIfAbsent(resourceID, id -> new EventFilterWindow(ongoingReList)); | ||
| ed.increaseActiveUpdates(); | ||
| log.debug( | ||
| "startEventFilteringModify: id={}, windowReused={}, ongoingReList={}", | ||
| resourceID, | ||
| existing != null, | ||
| ongoingReList); | ||
| } | ||
|
|
||
| public synchronized Optional<ExtendedResourceEvent> doneEventFilterModify(ResourceID resourceID) { | ||
| var ed = eventFilterWindows.get(resourceID); | ||
| if (ed == null) { | ||
| log.debug("doneEventFilterModify: no window for id={}", resourceID); | ||
| return Optional.empty(); | ||
| } | ||
| ed.decreaseActiveUpdates(); | ||
| log.debug("doneEventFilterModify: id={}", resourceID); | ||
| return check(ed, resourceID); | ||
| } | ||
|
|
||
| public synchronized Optional<ExtendedResourceEvent> processEvent( | ||
| ResourceID resourceId, ExtendedResourceEvent extendedResourceEvent) { | ||
| var ed = eventFilterWindows.get(resourceId); | ||
| if (ed != null) { | ||
| log.debug( | ||
| "processEvent: buffering event in window. id={}, action={}, rv={}", | ||
| resourceId, | ||
| extendedResourceEvent.getAction(), | ||
| extendedResourceEvent | ||
| .getResource() | ||
| .map(r -> r.getMetadata().getResourceVersion()) | ||
| .orElse("?")); | ||
| ed.addRelatedEvent(extendedResourceEvent); | ||
| return check(ed, resourceId); | ||
| } else { | ||
| log.debug( | ||
| "processEvent: no active window, surfacing directly. id={}, action={}", | ||
| resourceId, | ||
| extendedResourceEvent.getAction()); | ||
| return Optional.of(extendedResourceEvent); | ||
| } | ||
| } | ||
|
|
||
| private Optional<ExtendedResourceEvent> check( | ||
| EventFilterWindow eventFilterWindow, ResourceID resourceID) { | ||
| var res = eventFilterWindow.check(); | ||
| if (eventFilterWindow.canBeRemoved()) { | ||
| log.debug("Removing empty event filter window. id={}", resourceID); | ||
| eventFilterWindows.remove(resourceID); | ||
| } | ||
| return res; | ||
| } | ||
|
|
||
| public synchronized void addToOwnResourceVersions(ResourceID resourceId, String resourceVersion) { | ||
| var window = eventFilterWindows.get(resourceId); | ||
| if (window != null) { | ||
| log.debug("Recording own resourceVersion. id={}, rv={}", resourceId, resourceVersion); | ||
| window.addToOwnUpdateVersions(resourceVersion); | ||
| } else { | ||
| log.debug( | ||
| "addToOwnResourceVersions: no active window for id={}, rv={} (skipped)", | ||
| resourceId, | ||
| resourceVersion); | ||
| } | ||
| } | ||
|
|
||
| public synchronized void handleGhostResourceRemoval(ResourceID resourceId) { | ||
| log.debug("Ghost resource removal: discarding event filter window. id={}", resourceId); | ||
| eventFilterWindows.remove(resourceId); | ||
| } | ||
|
|
||
| // for testing purposes | ||
| synchronized Map<ResourceID, EventFilterWindow> getEventFilterWindows() { | ||
| return eventFilterWindows; | ||
| } | ||
|
|
||
| public synchronized void setStartingReList() { | ||
| log.debug("ReList starting: tagging {} active window(s)", eventFilterWindows.size()); | ||
| ongoingReList = true; | ||
| eventFilterWindows.values().forEach(EventFilterWindow::setReListStarted); | ||
| } | ||
|
|
||
| public synchronized void setRelistFinished() { | ||
| log.debug("ReList finished: clearing tag from {} active window(s)", eventFilterWindows.size()); | ||
| ongoingReList = false; | ||
| eventFilterWindows.values().forEach(EventFilterWindow::setReListFinished); | ||
| } | ||
|
|
||
| public synchronized boolean isActiveUpdateFor(ResourceID resourceId) { | ||
| return eventFilterWindows.containsKey(resourceId); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would help to include the sequence diagram that is in the PR here as well… Actually, a sequence diagram for each of these cases would be nice but might be too much work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, also that blog post I would like to keep short, so folks actually read it. The unit tests basically cover all the sequences, and there is >20 of them, what is quite a lot.