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

Handle only the latest event with the most up to date state #43

Merged
merged 1 commit into from
Apr 26, 2019

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Apr 26, 2019

Issue : #42

With this change, if the queue contains a batch of events, react only to the latest one — to follow the "eventual consistency" principle of Kubernetes.

The batch is time-framed to 0.1s, so that it is fast enough for all normal cases when only one event arrives, but slow enough for the initial object listing when multiple events arrive.

Still, some minimal time-frame is needed, as the streaming and parsing of the events inside of the kubernetes library is not immediate.

@nolar nolar added the bug Something isn't working label Apr 26, 2019
@nolar nolar requested a review from samurang87 as a code owner April 26, 2019 08:41
@zincr
Copy link

zincr bot commented Apr 26, 2019

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@zincr
Copy link

zincr bot commented Apr 26, 2019

🤖 zincr found 1 problem , 0 warnings

❌ Approvals
✅ Large Commits
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

@samurang87
Copy link

So basically if the fetching manages to get any events, you restart it to make sure there is only one event fetched? 🤔

@nolar
Copy link
Contributor Author

nolar commented Apr 26, 2019

@samurang87 Yes. And getting all the events till nothing is left. All events except the last one are therefore ignored — as intended.

@parking52
Copy link
Contributor

Why will the event we are interested in be fetched rapidly ? And why will the number of event decrease ?

@nolar
Copy link
Contributor Author

nolar commented Apr 26, 2019

@parking52 @samurang87 I didn't get your last comments. What number of events decreased? What exactly is fetched rapidly?

@nolar
Copy link
Contributor Author

nolar commented Apr 26, 2019

@parking52 @samurang87 See the examples in #42 — there are few objects with few events (e.g. "mycrd-expr1"), of which only the latest is of our interest.

@nolar
Copy link
Contributor Author

nolar commented Apr 26, 2019

@parking52 @samurang87 This is efficiently an equivalent of this logic:

while queue.qsize() > 0:
    event = await queue.get()

Except that:

  • threading.Queue().qsize() on Mac OS (it is not supported), so I used to avoid qsize() and just to not think about it. It is promised to work normally in asyncio.Queue though.
  • If the queue is empty (qsize == 0), we should wait for 5.0 seconds until something appears. So, while queue.qsize() > 0: is not a suitable criterion here, unless first_time is introduced. But it makes it same as not nice as double tries. In that case, it would look like this:
event = None
first_time = True
try:
    while first_time or queue.qsize() > 0:
        first_time = False
        event = await asyncio.wait_for(queue.get(), timeout=5.0)
except asyncio.TimeoutError:
    if event is None:
        break

# when (event is not None) or (no timeout on the last event), continue.

@nolar nolar merged commit ec6fc27 into zalando-incubator:master Apr 26, 2019
@nolar nolar deleted the 42-ignore-past branch April 26, 2019 15:15
@nolar nolar added this to the 1.0 milestone Apr 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants