Magento Architectural Discussions, 22 May 2019

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Magento Architectural Discussions -- May, 22, 2019

Description

- Compiled interceptors
- Immutable DTO
- Fix for locking during cache generation

Meeting minutes - https://github.com/magento/architecture/issues/164

A

Let's again like do you have any conversationalist and she said from reality, or you just want to bring this topic to our attention. I just.

B

Want to bring this topic to your attention. Okay,.

A

So like in any case like we already like sync up with logistic- and we will have their meeting today later- probably in two or three hours and more will even eat some code review for it, but just like or better visibility will be great. If you don't make the presentation as a request.

A

A

I'm still waiting for my sheets to confirm the timeline problem.

B

Okay, so basically the pull request is introducing anyway of generation interceptors with Eagle to move.

B

To move load from the runtime to the basis generation of the interceptors, so in result the Interceptor methods will not rely on configuration, but we'll have all the plug all the plugins and methods included in the generated code.

B

B

Have seen draw already used by even cheaper name, that is present on the call unreal and eager thanks for every as well yesterday and yeah. That is achieved by replacing the generator.

B

The generator generator and related infrastructure files, the only concern that I see, is that we are losing dynamics so in developer mode, we'll have to regenerate the interceptors each time a plug-in is added is introduced.

B

Also, we do have kind of biting benchmark, provided in the pull request, as you can see, from white benchmark, the first request, or the generation of the interceptors will be slowed down after this, but we'll have.

C

D

What's the difference between this proposal, I compiled.

A

So we can please open the result of the generation.

D

And the main benefit which we are trying to achieve is just to have a code in place.

D

And it's only for the scepters I.

E

Have a small comment from a community perspective, so let's say this is plug-in in my opinion, like apart from performance reasons like a performance issue is not as big one here, but the biggest one is actually debugging. So after applying this one, you actually can directly see without stuck trace.

E

Look up what happens and let's say, if you open a Blackfyre on your local and run some profiling with this mold on, you can actually see the actual methods of plugins in a stack trace of black fire instead of having called plugins magic from the standard, magenta interceptors, it makes debugging very complex, yeah.

A

Makes sense so actually that's what we wanted to leave because idea, whether we need to segregate that for the production mode and because, like we, every considering that the main benefit of this for us is to experience ruin the dev experience whether it makes sense to proceed with the same at orientation of the plugin.

C

C

F

Do we know how much time the generation will take for the developer mode.

F

So I suppose even further develop our Mortes pros and cons. The pros is that the debugging is easier, but then the con is that you know then becomes dependent on and I'm, not sure. You know like.

F

When they're making changes.

C

G

Completion option.

B

So there are two drawbacks for development developer mode. First, you need to clean clean their generated code. Maybe it can be addressed by automatic, clean or providing a command-line tool. Is somebody suggested in the pull request, and the second is that right now we also interceptors.

B

For now just replaces existing generation, the command-line utility to clean and regenerate the existing generated files and develop from what is just a proposal, and it's currently not included trees as a pull request.

D

F

E

I was actually proposing on this poor request. I was proposing to create file Watchers that watches the file being changed on developer computer and clean the generated classes related to this file based on the AI configuration, but nothing about actually recompiling. So idea behind. This is actually just to make sure that, when developer refreshes the browser window, the file that is not generated will be generated automatically by magenta for the standard means without any changes into the way how its compiled or how compiled classes are generated.

E

It just the developer tool for making sure when you develop, you don't have to clean generate in the classes yourself. You can just run a tool as the file washer and it will clean it for you.

E

But it doesn't mean, like yeah NFS, actually, it's stupid to use NFS for your development, because NFS is super slow with number of files. Magenta has so what I've seen so far in community like there is multiple developers who use different tools like people use units on file synchronization or this new tool that is available over there. I.

E

Don't remember me John yeah, but the idea is right to run it on your development machine and generate also classes also to have this one synchronized through unison or any other means, with your show machine where you run Magento itself, but in general, like majority of the distance right now, have possibilities to use inotify or queue pull solutions to pull up for possible file, changes and yeah. It requires, let's say developer to install some PHP extensions.

E

It will provide this solution, but it's not required to be like the requirement for wall magenta development like it's additional tools. If person wants to.

A

A

So frequently they're.

A

A

H

Because I think it's significant.

B

Yeah I agree. Probably it will be also beneficial to outline all the cases when generators should be cleaned.

A

Okay, we can proceed to another question. Yes,.

A

Here you gargle.

I

Can you hear me now yeah yeah great, so you should also see my screen. So the point is.

F

I

I

Screen, ok screen one okay now should be working, so the point is to add one immutable. Dt of support in Magento and I've been working on this one for for a while, and basically the the idea is just to work with immutable. Lead videos like this one, you you can see my screen, so basically parameters are injected through constructor and you olia get parameters. I started working on my class that basically should replace the data object helper that is capable of dealing with this kind of classes.

I

So basically you can use an approach like this one I'm going to show you so you.

I

Create from yeah, so basically you can create from array 1d to class and basically PSD to your class is being generated. This was the easy part. The hard part is basically to integrate this behavior with the existing framework. I will try to explain you why?

I

Because now most of the objects image enter using this method to be populated, that is populated with array that basically relies on sectors and of course we cannot rely on setters if we are dealing with immutable details and now what I'm trying to do and probably I need some good idea about this is how to make sure to basically use my nudity processor for let's say, extension attitude.

I

So, let's imagine we have a waiver imagine to class like one product so and we want to adding an extension attribute, and this extension attribute is a one immutable DT, oh, if I try to do this in the correct magenta plantation, basically, I have a problem because everything is handled by populate with array in a recursive way. So even extension, artists are being populated.

I

With this system, I try to create some kind of hybrid approach, I breed approach by creating what we find our hydration strategy, but basically is the feature of the DTO processor to be able to inject both mutable and immutable objects. But the problem here is that I should replace every single implementation of populate with array in a magenta code and of course, it's almost impossible, because I think I can have a lot of side effects, and this is one problem.

I

I think we should discuss about the implementation of the mutable DTO, because I need to find a way to identify. One immutable object is in magenta to be sure to use the right approach populate with array or create from array or the mutable and plus I also introduced that kind of as a kind of PS 37.

I

So basically, when dealing with immutables, of course, I cannot change the mutable, but I can create a copy of the object with updated information and if you follow the PS r7, basically every time you call one with fun and with function, basically you're creating one copy of the object with one single parameter updated.

I

If we bring this approach to Magento, basically I think we can blow up every single server of Magento, because the objects are quite wide, so I ended up creating kind of I call this DT or mutator. That basically is a generated class, so I'm using cogeneration. So let's say I want to mutate. This object. That's nested, video I, just pulled test. Nested, eto, mutator and I have all the Gators, basically with equivalent with metered, but instead of having one copy of the object. Every time, I call weed, I call a final mutate.

I

So it's like a builder approach, and this is what I'm working on and the main problem now is how to integrate this in the current merchant implementation. Because of the populate with our emitter, the was talking about before.

I

Yes, we have been thinking to a marker interface, but my concern about a marker interface that actually I have a branch with with a marker interface, because I've been running a couple of tests is that we are I mean it's a kind of smell, because basically we cannot say that one object implementing this interface is actually an immutable object. I know we can apply, let's say some static test, but you know it's not a code constrained.

G

This very very straightforward, they told in your opinion, does it make sense to move definition of this object into configuration. It will resolve both of problems because we can generate not only builder, which is material I, believe we will be able to generate data transfer object.

G

Also, if you resolve patient with the market interface, because in case we have a configuration which has information about all our data objects, it's pretty straightforward, pretty nice purpose of it and how.

I

We can this was another idea. Actually they've been sharing some months ago with with Igor, actually was slightly different, but basically it was an XML file, defining the DTO there's no more and it was auto-generated that was basically I. Don't know if you mean this way, so you define an XML defining the DTO yep.

G

Actually, I believe that I.

I

G

Extension attributes in some kind, so we can extend this game.

I

So absolutely it's an option. It's an option, but of course you have XML and not yeah I mean, of course you generate a class, but you have an X term and instead of V I, don't know how this is appreciated from mature to developers. So this is. This is absolutely one idea and I've been even working on this approach, because I have several.

I

The point is not Excel is, it is not PHP is.

A

The point the correct way.

A

Using this proposal see, we can create the immutable data already now, because this proposal is backwards compatible and the wrong with that doesn't actually introduce new API, probably except the marker interface or like we can proceed in the result. Marker interface, but then you declaration is going to be kind of new from genomic in your API and we can't relieve it before to the for, but already now we have some acct to introduce mutable objects.

G

In my thingy, it really depends in case we are going to switch the configuration it's a moment. It makes sense to combine positive approaches because in other in other hand, we don't have that differ in the several approaches. We have to build in beautiful objects now system and- and it shows you that this will be good anyway, it in a new feature or I believe it can be approved as a backward compatible change, because no any of extension will be affected by introducing a new configuration and the new people. Oh.

A

Any questions regarding like the it.

C

Is it looks good, yeah question? It might be.

C

C

Jimmy I'm sorry I mean it.

C

Was it vindicates is provide? This is kind of provoking question yep.

A

Okay, so like this cool idea, like we already discussed this for you from the very beginning with Ricardo, but currently like this was probably several months ago, but now we get we good, but this year in the scope of the implementation of the install become concurrently like we have a higher architecture. We have the amplitude sources and the sources, but then actually any source, which could be beautiful. It can represent like warehouse with mortars, poor distribution center, actually, whatever.

C

My question like.

A

No, no! No! No! We are not.

A

Okay, yeah, so just to emphasize this scene, we are not trying to refactor or migrate all the existing objects, so we just want to provide a possibility for the new orbit if this object need to be created as immutable to be created as immutable. Actually, that's only thing we want to it and.

A

I

So basically, this is. This is not a new way of thinking to objects, of course, or it is not a fight between mutable and immutable objects. I mean you should use mutable when you should, when you have to use mutable and immutable when you have to use immutable, but for example, now, if you take the request, interface object, it's just an example. Why request interface object is mutable.

I

So basically, we sometimes we need to create immutable objects, but now, today a majority is totally missing. This aspect I think.

I

I have just one question for you, because I am quite concerned about this approach with this kind of let's say kind of PSR 7, because I, don't like the Builder approach, so I want to know what is ready. If this is acceptable or I should be change, work.

G

So they can. I clarify the question: why sometimes.

I

G

Can you clarify the question yeah.

I

So, basically, when I, when you want to change the information of an immutable object, you have to create a copy of the middle of the immutable object.

G

You created I thought if interruption, maybe my concerns, that's why we are named a builder as I'm a teacher but I, don't know I.

G

My personal opinion I prefer to have a builder with the prototype, because for some reason we you may want to change one argument for what well for one for another case, you probably will want to create a new object from prototype. I, don't know.

F

I

This is not a builder I mean this is to change the information of an existing object and not to create the object, because, if you want create the object you can create, you can do create from array or using a factory and injecting the parameter as you like.

I

This is basically consider this is almost the same of set IDE in the mutable approach. Let's one.

G

One case when you have to put your data object as an argument for another plugins, which probably wants to change its state. The main problem. This builder, am pleased with this change in state that we always face when we try to do something like this was exactly in this case that you may have several extensions that may want to change your objects on fly during execution right.

I

In this case, it should not be readable.

G

But in case we don't have a mutable builder and immutable objects. We probably may say that's okay. We have only one event in our system, which is builder, create which allows you to put your extension logic on it in other hand, all other logic in our system will play with some visible objects.

I

G

That this is just proposal to consider it I, don't know if you're interested in we can continue this discussion of wine absolutely yeah.

I

Yes, you can see hi everyone working progress branch here. That is the request.

F

I

Two to five double four I'm working on these, but, of course it's a work in progress, so yeah.

I

A

A

And the main point here is how we like how we want to you. Take modified our immutable object right. Whether we need to stick with these are seven, whether we need to use the kind of factory based approach like.

J

Pretty much like Haas helps that you just calling the Saturn.

A

Let me write our proposal for the magenta to service levels was done like three years ago.

I

Yes, but if you, if embed logic in the DTO itself, basically you need to create a copy of the object for each single parameter, you're going to change. So the idea of this potato was to avoid these. So if you need to change, let's say like in this example: I'm sharing I want to change three parameters: I, don't need to have three copies: I, just well: I have one P when I call mutate. Oh, this is the main difference and I want to discuss about these.

I

So if you think this is correct or if you prefer to have a PS 37 pure approach, but.

C

I

Think it can be quite confusing from the.

A

D

You don't over, like forever wait yeah. We are not going to.

C

Keep, but it's mostly about up to do is unusable state.

A

Okay, so let's proceed with reviewing Ricardo's pull request and we discuss the next question. Probably the Cascade about the four support. Two options like mutable object, fully and the desire an approach and like how to discuss the way how's. The haveno mutate immutable.

A

The last think we Ricardo for your presentation, the last but not least today is you want you for me, I.

A

Partly believe that there would be time of day, one.

A

Why do you can start reading? You know making presentation.

E

Shall I start or yeah yeah, okay, so basically I just want to I think we had a little bit of discussion with Andrey in pull request. So basically, as a current problem will we have is that there is looking.

E

Wait a second yeah. Let me share just my complete screen.

E

Okay, do you see my screen? Yes, oh sorry, I need to close this one out. Okay, so here's my screen very interesting one. So the problem right now is around the way how, as the cash is getting loaded in magenta, because in recent 2.3 release there was introduced a way to walk cash during writing to it. So let's say when you have a lot of concurrent users and then someone triggers the clashing of the system cash.

E

All the concurrent users are getting locked out of accessing the cache and then yeah, let's just open the example and let me actually switch to the original range and stash my changes yet.

E

Okay, so now I can just switch to the develop.

B

E

So the current issue is around this lowering of the data, so there is right now used method, called locked load, data and itself. So first it tries to read the data from cache.

E

Is the data is not in cached and the data is locked, it will put the process to sleep and it will try to read the data until the lock is released and at the same time afterwards it tries to acquire the lock until it can acquire it as I see here, for instance, the other is a while loop until the cache data is actually retreat.

E

So in cases where, let's say you have a cat be not generated, yet it puts your Magento request with concurrent requests. At the same time, let's say in developer mode, where you have also to get PHP involved for static content and everything else. It puts all those processes to sleep until the cache data gets gonna raid, so developers when they developed something. They feel a lot of lag in.

E

Exiting the store, however, yes, there is some merits in actually using logs, because I was working a lot with merchants who have had this problem before in magenta one a lot especially, and we had to patch the mage core config model that in a racist cache. In order to make sure that, during the write operation we acquire a lock and when we acquire a lock, the other processes are not gonna overwrite.

E

The data after our lock is happening so before like there was implementation that was locking the configuration, but it was not checking if other process just was working before you actually tried to check if it was locked before so it's a little bit complicated the implementations. It was happening in magenta, one in magenta. It was, it is solved in such a way what I was doing for magenta one customers.

E

We were just checking initially if the process was locked.

E

Actually, if the cache entry was locked, we were checking it initially and then, after the initial check was happening, we were storing it in a variable and when we were generating the data we were checking after the data is generated. If the process was logged before we started getting the data.

E

Into the storage.

K

K

Because they were falling behind.

K

K

Always right, I think.

K

K

K

That process right sketch all everybody else can read it now. That is a problem with that implementation. My understanding I'm not sure about all the problems, but at least one that I noticed one. That I think that mood in the if gosh is disabled. I think that.

K

Someone had I think cache disabled, which caused every every every process to wait for lock and then regenerate, because capital is disabled. So now so it.

E

It's it's true and I'm. Sorry for interrupting you, but yeah! You are completely correct. Okay,.

K

Good so now we are discussing hello.

K

So now we are discussing about cell discussion is about removing this second level optimization because it causes more problems than it solves, so fall back to just having a lock on single process and not preventing all other processes from reading. So every process will still read the configuration and match it and process it, but will not write to care. So we will still load our CPU and our phone system, but that's not a big deal, it's much better than introducing locks that will cause problems in some scenarios right overall description of the problem.

K

What we're trying to solve right, yeah.

E

So it's basically a good description. I couldn't put it better than you, so in general, this is apart from Z. There is terrific theoretical possibility that under high load, when, let's say the car generation would take a lot of time that all the backend requests that run concurrently will actually wait for this one to be completed by depleting the FBM process pool on a server so ballad. This is I was unable so far to replicate with load test, because I don't have.

K

I think it makes sense to remove this lock, because we know that least one scenario, but it practically causes problems. We could think about solution to solving the scenario where caches are disabled by Y. Do that? If the benefit of the solution is not such a big now so I think we have. Everybody agrees that it should be removed right.

C

Not so big button like 20 concurrent users, user, not its generation, okay,.

K

So so I think we have general conceptual agreement.

E

K

For the discussion is just theoretical right and.

E

It's it's the theoretical about another issue that could become.

K

K

They can, they can start, they can consume all fpm processes under high load right yeah.

E

And and then it they will be unlocked as soon as the caches gonna rate it and store it into the cache storage. But it makes a huge possibility is that you know, like our server, just waits for cache being generated.

E

In some case, the cache server like radius, let's say, goes down during the write operation. It means yeah and until you know the main process fails. You know and then locks the cache we are here. It's finally is working, but let's say if this situation happens and somewhere the cache server goes down. It means that then the same problem as in developer mode will happen on production as well, because, let's say.

E

Let me just apply some of the changes I was doing so here is the standard implementation. So this one I, don't I, didn't touch in my pool request, so I created another method under the lock cache guard class called non-working, lock. Claude said that's the things that you just described, that we just checked for the lock and then, if the process was locked, we only in this case we save the data.

E

If the process is not worked and we can acquire the lock otherwise, we just return the data that was collected back to the user, so the process works well without you know generating it again. Apart from that, after some discussions with Andrea I was thinking. How can we actually improve the generation process and I created? You call a book here called stale beta loader, so let's say we can retrieve the data from cache.

E

We just returned it back if it's locked and we have a stale data loader, then we retrieve data with this stale data loader back to the user other ways. Otherwise we will get irate the data through the data collector itself. So the basic idea here is that when we save the original cache, we create as well another entries that will contain a stale cache that we can use to retrieve data during generation. For all the other processes, there was valid concern from Andrea concerning the page, cache and other parts of the systems. It can be.

E

How do I call compromised by the inconsistent data? So let's say you can store the page cache entry in warnings that would still be served for a long time with this stale, cached data and produced and, of course, the wrong date on the front end. However, this one can be solved by just disabling the page gate for the pages that received stale configuration cache, so it just can be resolved through the plug-in, and another issue was let's say when we have other caches apart from page cache like what's a block ECML cache or collection cache.

E

That also will rely on configuration. Well, yes, but I already found out that Magento doesn't solve this problem even right now, because if I change the configuration value and flush, this configuration cash at the moment and I have to say in other cache. That depends on this configuration value that other cache is not clutched in Magento, so I was able to replicate it by enabling price block, HTML cache, changing, let's say the currency code and removing the currency code from this block.

E

Html cache key and the problem was reproduced very easily like on John 2000, clockwork HTML key one with flash configuration cache. So it's very simple one to understand, I, think and then we have already a solution for such cases. Let's say if your data depends on other parameters like what's a currency code or the tax calculation functionality like tax configuring, tax calculation, gradation or anything else from outside of your internal data that has been returned to be cached. You usually just add this data as a part of the cache key.

E

So when that other data is changing, you would create, let's say, a new cache key entry, and then you will never get stale cached data for your functionality, so this is actually happens with price, a block, HTML, cache and I. Don't think like. We will have a big problem if we just serve some tail country cash, while other processes, while the main process writes data to cash, we just returned back the other ones. So it's going to be like.

E

E

So the thing is that the stale cash is written to. Let me just show you an example, so we have system cash here and here we in a cached data we just save the main config type, and then we also save this stale cash.

E

It's out so this stale cash gets saved during a single locked process. So idea is that we only lost one time and we only logged you in right operation. So we don't lock it during the read and we don't wait for the walk on the read operation. So we have only one writer is responsible for entry into the.

C

C

E

So in case, if a stale cash is not there, it will just return false and it means it will invoke the data generator.

E

So it's as simple as that, so it's it's also will work in the same way in disable cache mode. So when we have disabled cache right, we don't save cash into the cache storage and in this case, like.

E

Generated data, not the cached one.

E

So it's it's. Basically, it's a role removing in the pool request, cool, thank.

A

You one just just my two cents to this.

A

A

A

A

E

Also, just a quick question for you, guys is if anyone is able to help me out with a small bug, I encountered in putting a plug-in on the system cache. So maybe someone can help me out with that one, because it's kind of a blocker for my finishing of the pool request. Can someone assist me with it.

K

Yes, definitely someone canister, either I think will be the best.

A

K

A

Can like we definitely can calculate you with guys from the community engineering team I see that Sergei is still on the call. So is this kind of yeah.

K

But we can, you can just ask it in our design, chat or find someone most problem like you find. This is the problem that we described correctly this pro most, probably that object is required for plug in plug in mechanisms at why it have circular dependency, but.

E

Okay, thank you very much.