Hi Alex, I did not take times (which depends on a number of variables that drastically change between environments), but verify that the number of updates has been reduced drastically without losing functionality, which is objectively a good thing. If before the change, for every node executed, we have an update for every node previously executed, so if a process have 50 nodes to execute, we were performing nearly 50*51/2 updates, which gives us a total of 1275 updates, now we have just one for every node being executed, implying a total of 50 updates.
On Wed, Jan 17, 2024 at 3:18 PM Alex Porcelli <a...@porcelli.me> wrote: > Francisco, > > I noticed that your PR has been merged, but I was expecting (at least > was my understanding from this thread) that before merging some > benchmark data would be shared in advance - to assess the cost/benefit > of such a decent size change. > > Do you have any information to share? > > On Sat, Dec 23, 2023 at 4:02 AM Francisco Javier Tirado Sarti > <ftira...@redhat.com> wrote: > > > > Yes, as intended, now we have one select and one insert/update per node > > event. > > I moved the PR as ready for review and give @Pere Fernandez Perez > > <pefer...@redhat.com> permission to the branch so he can edit it in the > > next two weeks (Ill be on PTO) if desired, before merging. > > > > > > On Thu, Dec 21, 2023 at 5:58 PM Alex Porcelli <a...@porcelli.me> wrote: > > > > > Cool, thank you Francisco! > > > > > > Did you manage to get some preliminary data about improvements? > > > > > > On Thu, Dec 21, 2023 at 11:52 AM Francisco Javier Tirado Sarti > > > <ftira...@redhat.com> wrote: > > > > > > > > Yes, after some delay because of quarkus 3 migration. Im refining > this > > > > draft PR > > > > https://github.com/apache/incubator-kie-kogito-apps/pull/1941 > > > > > > > > On Thu, Dec 21, 2023 at 5:48 PM Alex Porcelli <a...@porcelli.me> > wrote: > > > > > > > > > Any update or new findings on this topic? > > > > > > > > > > On Tue, Nov 28, 2023 at 8:38 AM Francisco Javier Tirado Sarti > > > > > <ftira...@redhat.com> wrote: > > > > > > > > > > > > Hi Alex, > > > > > > After considering different options to improve performance, we > feel > > > that > > > > > it > > > > > > is time to "partially" move away from the current Map style > > > interface ( > > > > > > > > > > > > > > > https://github.com/apache/incubator-kie-kogito-apps/blob/main/persistence-commons/persistence-commons-api/src/main/java/org/kie/kogito/persistence/api/Storage.java > > > > > ) > > > > > > which was shared with Trusty, to one more suitable for usage > with a > > > > > > relational DB like postgresql (but still compatible with big > table > > > dbs). > > > > > > The idea will be to replace generic Storage interface by four > > > specific > > > > > > interfaces (which will inherit from a common one that keeps the > query > > > > > part > > > > > > at is it. with get and query methods), that will include the > required > > > > > > modification operations for the four DataIndex "domains": > > > > > processinstance, > > > > > > usertask, processdefinitions and jobs. Those interfaces will > define > > > > > methods > > > > > > like addNode, addVariable, updateTask, addAttachment..... that > will > > > allow > > > > > > the persistent layer implementation to just update the needed > info > > > in > > > > > the > > > > > > DB (for example, for addNode in Postgres, just insert a row into > > > nodes > > > > > > table, for addNode in Mongo, basically the same atomic upsert > > > operation > > > > > > that is currently done). Therefore, we increase performance for > > > Postgres > > > > > > and keep the current one for Mongo. The current DB schemas won't > be > > > > > > touched. > > > > > > Since the code change is large, I do not think I'll be able to > have > > > the > > > > > PR > > > > > > ready till next week. > > > > > > But before starting, please let me know if that approach is fine > for > > > you. > > > > > > Best regards. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Nov 24, 2023 at 6:55 PM Alex Porcelli <a...@porcelli.me> > > > wrote: > > > > > > > > > > > > > Thank you Francisco to getting deeper on this… > > > > > > > > > > > > > > Looking forward to see the results of your suggested > improvements. > > > > > > > > > > > > > > > > > > > > > On Fri, Nov 24, 2023 at 9:40 AM Francisco Javier Tirado Sarti < > > > > > > > ftira...@redhat.com> wrote: > > > > > > > > > > > > > > > I forgot to attach the queries > > > > > > > > > > > > > > > > On Fri, Nov 24, 2023 at 3:04 PM Francisco Javier Tirado > Sarti < > > > > > > > > ftira...@redhat.com> wrote: > > > > > > > > > > > > > > > >> Hi, > > > > > > > >> A brief update on this topic. > > > > > > > >> After doing a simple test with example > > > > > > > >> > > > > > > > > > > > > > > > > https://github.com/apache/incubator-kie-kogito-examples/tree/stable/serverless-workflow-examples/serverless-workflow-data-index-quarkus > > > > > > > , > > > > > > > >> the number of updates over Nodes table is n*n, so we manage > to > > > > > obtain a > > > > > > > >> perfect quadratic performance degradation. The problem is > worse > > > in > > > > > the > > > > > > > case > > > > > > > >> of Serverless Workflow than in BPMN because we the number of > > > nodes > > > > > is > > > > > > > >> greater than the number of states. In that example N is 16, > but > > > for > > > > > a > > > > > > > more > > > > > > > >> complex workflow it would be certainly large. > > > > > > > >> I think that this is more related to how we are handling > JPA in > > > the > > > > > > > code, > > > > > > > >> in particular the mapping from model to entity (basically > JPA is > > > > > blind > > > > > > > and > > > > > > > >> has to update all nodes for every write because it believes > the > > > > > node has > > > > > > > >> been updated, although it is not) than an issue in the table > > > > > definition. > > > > > > > >> In fact, when using JPA, separating the server model from > the > > > JPA > > > > > > > entity is > > > > > > > >> not a good idea, especially if the entity contains > collections. > > > I > > > > > will > > > > > > > try > > > > > > > >> to change that without breaking anything. > > > > > > > >> > > > > > > > >> On Wed, Nov 22, 2023 at 12:10 PM Enrique Gonzalez Martinez < > > > > > > > >> egonza...@apache.org> wrote: > > > > > > > >> > > > > > > > >>> After the events split you now will need to create a node > > > instance > > > > > > > >>> model instance of making independent from the process > instance. > > > > > > > >>> That should do the trick. > > > > > > > >>> > > > > > > > >>> Regarding deleting/inserting it was fixed at some point. > > > > > > > >>> > > > > > > > >>> El mar, 21 nov 2023 a las 20:22, Francisco Javier Tirado > Sarti > > > > > > > >>> (<ftira...@redhat.com>) escribió: > > > > > > > >>> > > > > > > > > >>> > Hi Martin, > > > > > > > >>> > I have a task to review performance of > > > > > > > >>> > > > > > > > > >>> > ProcessInstanceNodeDataEventMerger > > > > > > > >>> > My idea is to reduce the number of delete inserts when > > > processing > > > > > > > >>> events > > > > > > > >>> > and try to do it incremental. > > > > > > > >>> > That should improve performance. > > > > > > > >>> > PS: > > > > > > > >>> > I was planning to send an e-mail tomorrow announcing > that in > > > > > case you > > > > > > > >>> were > > > > > > > >>> > already working on a fix for that. I assume you are not > and I > > > > > would > > > > > > > be > > > > > > > >>> > sending a PR soon. > > > > > > > >>> > > > > > > > > >>> > On Tue, Nov 21, 2023 at 6:09 PM Martin Weiler > > > > > > > <mwei...@ibm.com.invalid > > > > > > > >>> > > > > > > > > >>> > wrote: > > > > > > > >>> > > > > > > > > >>> > > I looked into the new examples using data-index > persistence > > > > > addon - > > > > > > > >>> Neus' > > > > > > > >>> > > PR#1813 [1] for serverless and Pere's branch [2] for > > > workflow > > > > > > > (great > > > > > > > >>> job > > > > > > > >>> > > both!) - and they work without issues using single > > > requests. > > > > > > > >>> However, under > > > > > > > >>> > > some load (I used 'ab' for testing with a light > > > concurrency of > > > > > 10 > > > > > > > >>> parallel > > > > > > > >>> > > requests) I ran into the following problems: > > > > > > > >>> > > > > > > > > > >>> > > (1) Large number of insert/delete calls (eg. for tables > > > such as > > > > > > > >>> nodes, > > > > > > > >>> > > definitions, etc) > > > > > > > >>> > > > > > > > > > >>> > > (2) Hibernate OptimisticLockExceptions / > > > StaleStateExceptions > > > > > > > >>> > > > > > > > > > >>> > > (3) DB deadlocks > > > > > > > >>> > > > > > > > > > >>> > > (4) Error responses, slow response times > > > > > > > >>> > > > > > > > > > >>> > > The reason I am reaching out with this topic here is to > > > find > > > > > out if > > > > > > > >>> we are > > > > > > > >>> > > aware of this issue, and if someone is already looking > > > into or > > > > > > > being > > > > > > > >>> > > assigned to it? > > > > > > > >>> > > > > > > > > > >>> > > Thanks, > > > > > > > >>> > > Martin > > > > > > > >>> > > > > > > > > > >>> > > [1] > > > > > > > >>> > > > https://github.com/apache/incubator-kie-kogito-examples/pull/1813 > > > > > > > >>> > > [2] > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > > > > > > > > https://github.com/pefernan/kogito-examples/tree/example_data-index_persistence > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > > > >>> > > To unsubscribe, e-mail: dev-unsubscr...@kie.apache.org > > > > > > > >>> > > For additional commands, e-mail: > dev-h...@kie.apache.org > > > > > > > >>> > > > > > > > > > >>> > > > > > > > > > >>> > > > > > > > >>> > > > > > > --------------------------------------------------------------------- > > > > > > > >>> To unsubscribe, e-mail: dev-unsubscr...@kie.apache.org > > > > > > > >>> For additional commands, e-mail: dev-h...@kie.apache.org > > > > > > > >>> > > > > > > > >>> > > > > > > > > > > > --------------------------------------------------------------------- > > > > > > > > To unsubscribe, e-mail: dev-unsubscr...@kie.apache.org > > > > > > > > For additional commands, e-mail: dev-h...@kie.apache.org > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > > To unsubscribe, e-mail: dev-unsubscr...@kie.apache.org > > > > > For additional commands, e-mail: dev-h...@kie.apache.org > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@kie.apache.org > > > For additional commands, e-mail: dev-h...@kie.apache.org > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@kie.apache.org > For additional commands, e-mail: dev-h...@kie.apache.org > >
