Skip to content
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

MC-13864: Consumer always read config from memory #91

Merged
merged 5 commits into from
Mar 18, 2019

Conversation

vzabaznov
Copy link
Contributor

@vzabaznov vzabaznov commented Feb 19, 2019

Problem

Right now our consumer depends on config/objects in-memory state. After application was bootstrapped it initialize a lot of object that are always kept in memory after first load - config, websites, etc.., since our consumers are long-living and their live time limited only by message count it leads to wrong im-memory config/objects states and we need to introduce the way to reinit them

Solution

Introduce poison pill - some message stored in Data Storage that signals that consumer must be reinited

Requested Reviewers

@kandy @paliarush @vrann

@vzabaznov vzabaznov mentioned this pull request Feb 19, 2019
@melnikovi melnikovi self-requested a review February 19, 2019 19:48

#### Extension Points and Scenarios

We will put poison pill into Data Storage based on common Magento extension points like events and plugins
Copy link
Member

@melnikovi melnikovi Feb 19, 2019

Choose a reason for hiding this comment

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

Could you provide more details? How it should work when you add variable on Magento cloud?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding variable on cloud triggers redeploy, which will restart consumers by default

public function put();
```

Before process new message, consumer will compare version of PoisonPill in-memory state with latest in DB. If version is different consumer dies and next cron:run will start new instance of consumer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean that there will be at least one MySQL DB query for each consumed message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add warning to the design document for the team implementing this feature to verify performance implications.

Copy link
Contributor

Choose a reason for hiding this comment

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

@paliarush @vzabaznov I am concerned about this point specifically, if for each read we are hitting the database query, then what is the purpose of storing configurations in-memory? we might lose the benefits of in-memory optimizations. We should think differently here, on invalidating the cache on event based.

@paliarush paliarush self-assigned this Mar 18, 2019
@paliarush paliarush merged commit 753cf51 into magento:master Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants