topic Re: Thread-safe action rules for massive concurrency in Alfresco Forum

Thread/Transaction-safe action rules for massive concurrency

iru — Thu, 25 May 2017 15:47:14 GMT

Hi there,Got a task that seems easy but is getting us lots of headaches.Functionally speaking, what we need is to create a custom rule in a folder, that given a file that is being uploaded, moves it into another folder, based on its creation date.File enters: InboxFile is moved to: Folderized / yyy

Re: Thread-safe action rules for massive concurrency

afaust — Fri, 26 May 2017 10:00:40 GMT

There is a lot to unpack here.

First, creating new transactions within already running transactions will never work if the nested transaction needs to access data created in the outer one. This has nothing to do with AOP, just with transaction isolation on the DB between two uncommitted transactions.

Second, using the synchronized keyword is a very brutal way of dealing with concurrency issues and is rarely appropriate when trying to deal with data in concurrent transactions. Java-level synchronisation is primarily relevant when dealing with Java object-level state, not transactional data. In the worst case it can cause your entire solution to lock up for good.

Third, most (if not all) operations that are triggered by a user will already run in a retrying transaction, so normally there should be little need in a rule / action to use the retrying transaction helper. Only if you are dealing with custom web scripts, cron jobs or other custom code that - for some reason - is not already running in a transaction will you have to deal with retrying transaction helper directly.

Fourth, you should never use Thread.sleep or anything similar in business-level code. If you think you are dealing with timing issues, a sleep will usually just mask/hide the problem without really addressing it. Most of the time sleep is used as a brutal, poor-man's tool for synchronisation. Outside of technical utility libraries, sleep should be considered a "forbidden API".

I have implemented similar rules / behaviours for moving content in a specific structure upon upload and never had to deal with explicit transaction management for these. Default Alfresco retrying functionality took care of any conflicts and in the worst case, a file upload was processed two or three times on the server (resulting in a bit longer overall duration). It is essential that you do not try to deal with / work around / supress some specific exceptions that are triggered in concurrent situations (e.g. via try-catch). The retrying functionality relies on exceptions for concurrency issues to be propagated up the call chain and if you catch such an exception without rethrowing, Alfresco will not be able to deal with them properly.

Re: Thread-safe action rules for massive concurrency

iru — Fri, 26 May 2017 10:45:32 GMT

Hi Faust,

Thanks again for replying.

Where are new transactions supposed to be created if not under a running transaction? Meaning.. which are the uses cases prone to create new transactions? Understand that " not already running in a transaction" operations already run within an Alfresco transaction... or maybe I'm misunderstanding it with the JVM active thread concept.
The AOP thing was just to emphasize about the stacktrace error. I understand that the first one (the parent) is still uncommitted, and may occur a visibility error between parent-child ones.
Agree with you that using synchronized is more a java thing, and that is not a best-practice, but wanted to ensure that each thread enters the code sequentially, due to the requirement that task1 (folderized structe creation) must only run once per day/minute/...
Don't think a well written synchronized method would have to block your entire application.
In this case, the actions are not triggered by a front-end user, but from an API exposed to third party applications through an upload operation vía CMIS endpoints. We tried all this scenario without using transactions and they were not retried.
Also agree with you that sleeps/synchronizations don't ensure that the transaction committed, it was just a proof-of-concept.
Also agree that exceptions should be propagated upwards, and we do so, but in this case Alfresco does not help us dealing with the problem.

From all your response, the workaround that may work, is to leave the task2 in the rule, but take task 1 and verify it ourselves in the custom code, simplifying things and hopefully our problem, but that won't give response to this matter itself. Still confused why it works in a postgres-based environment, but not in mysql. Have rechecked the compatibility matrix and alfresco version, mysql version, mysql driver and generated tables engine is InnoDB, so everything is within parameters.

Re: Thread-safe action rules for massive concurrency

afaust — Fri, 26 May 2017 13:52:39 GMT

Unfortunately your description of how the functionality is implemented is a bit too abstract to fully comprehend the problem or be able to provide concrete recommendations.

"Where are new transactions supposed to be created if not under a running transaction?" - In 99.9% of cases there should be no need for nested transactions, so a "new" transaction will either be the top-level transaction created by Alfresco automatically when calling a standard API endpoint or the top-level transaction created by a developer writing some sort of asynchronous code (e.g. jobs). Calls to CMIS endpoints should always trigger a retrying transaction automatically which would be propagated to any action called from a rule (without "run in background"). Unfortunately your stack trace is too short to point out the responsible component / be aware of the context.

Re: Thread-safe action rules for massive concurrency (MySQL)

iru — Mon, 29 May 2017 12:15:50 GMT

Morning,

Have updated the discussion with the requested information, and the action-context.xml with the "extraExceptions" in order the RTH to retry the procedure, as if not, it won't do it (they're not considered retryable)
The client side it's a simple CMIS upload with Atompub binding, and ConcurrentestRunner for concurrency test.

Anyhow, we're going to test it on an SQLServer implementation too, as my suspicion is over some kind of bug... we'll see

Re: Thread-safe action rules for massive concurrency (MySQL)

afaust — Mon, 29 May 2017 15:08:29 GMT

Ok - so you are not using the default retryingTransactionHelper but a custom instance with some custom extra exceptions. This is not something that I would recommend. For one, most of the exceptions included are not exceptions where a retry will have a chance to "fix" anything - some of them are indicators of serious developer errors, e.g. regarding not validating input / trigger conditions. Basically you are retrying to try and work around (potentially your own) developer errors...
Never have I encountered a situation where the default retrying transaction helper would not suffice.

I also see some potential issue with using the FileFolderUtil.makeFolders utility. This will also traverse and retrieve folder paths that the current user is not allowed to access, and the result may be unusable due to restricted permissions.

One problem you face in both of your constellations is that the FileFolderService.exists() check in line 90/91 will/should always fail on the first try of the action when the target folder does not exist. This is because the new folder will be created in a nested transaction and will not be visible to the outside transaction for the exists() check. As a result of that (I assume) the outer transaction will rollback and retry the entire action.

The issue with the NonRootNodeWithoutParentsException looks like it may have something to do with incorrect isolation levels on the DB tier and/or a potential bug in the parentAssocs cache inside the NodeDAO. Unfortunately, that cache is a custom implementation and not one of the regular caches, so it may be prone to issues not noticed in any of the other caches.
Is the MySQL server configured with the right default transaction level, and with the configuration parameters as recommended in Alfresco documentation? Normally, before the exception is thrown, Alfresco will check the database to load the parents of a node if they have not been cached yet, and for a node that has just been uploaded it should be able to find the parents (unless some other custom code has messed with the DB in the meantime or the isolation level is not working properly).

Re: Thread-safe action rules for massive concurrency (MySQL)

iru — Tue, 30 May 2017 15:36:32 GMT

Thanks for the recommendations regarding the RTH and FileFolderUtil.makeFolders usage, will take them into consideration.

Regarding the FileFolderService.exists, seems not to fail due to target folder being the object that is returned from the previous RTH task (plus we're not seeing the trace placed in the else statement).

As shown in the traces, the problem occurs when we're trying to move the just uploaded document into the just created folder.
__

NonRootNodeWithoutParentsException its an odd one and could understand it's due to some cache bug, but what about the second one; InvalidNodeRefException: Node does not exist?
This, if I understood it correctly, is referring to the fact that the transaction has no context of the node that is being uploaded itself.
__

We've followed the instructions within the configuration documentation http://docs.alfresco.com/5.2/tasks/mysql-config.html

The only aspect it talks about transaction levels, is that the database tables are to use InnoDB, and that's done correctly by the population script of Alfresco itself.

Have also read http://docs.alfresco.com/4.0/concepts/mysql-config-settings.html

where it recommends to set the "innodb_locks_unsafe_for_binlog = 1"

I'm no DBA, but is there any specific "transaction level" or "configuration parameters" we might be missing?

As read, the default transaction isolation level for mysql innodb is REPEATABLE-READ.
__

Been going back to previous simpler implementations, taking into consideration your recommendation regarding "CMIS endpoints should always trigger a retrying transaction automatically which would be propagated to any action called from a rule", but still don't see the correct way to implement it.

If multiple threads enter the rule, and only one is to create the folder structure... how should we develop the rule thread-safely?

FileFolderUtils.makeFolders, for instance, should work, as it states in the javadoc: checks for the presence of, and creates as necessary, the folder structure in the provided path

But "FileExistsException" is thrown for non first threads that arrive, and CMIS RTH does not contemplate it as retryable.

- src: https://pastebin.com/nJwQwTmG
- error: (4 thread execution in concurrency, 1 ok, 3 fail) https://pastebin.com/raw/M23c5j8C

Could force a while statement, until node is created or resolved, but again we're going into deep mud.
It's silly.. but any recommendations on this? Will try anything x)

Best workaround up to the point, using 1 RTH for the folder creation, to be tested in sql server and hope for the best :rolleyes:

Re: Thread-safe action rules for massive concurrency (MySQL)

afaust — Tue, 30 May 2017 22:40:11 GMT

If you look at your own error stacktrace (https://pastebin.com/raw/yVNc8YWf ) you can see that a couple of lines above "org.apache.chemistry.opencmis.server.impl.atompub.CmisAtomPubServlet.service" there is an instance of "org.alfresco.repo.transaction.RetryingTransactionHelper.doInTransaction" triggered by the RetryingTransactionInterceptor. That is the retrying behaviour of the CMIS endpoint I am referring to - and this will apply to most (if not all) of the default Alfresco endpoints.

I wish you would stop referring to "thread-safety" - what you need/want is "transaction-safety", which is a completely separate technical concept, and by continuing to use "thread-safety" you risk confusing other people / community members that may stumble over this thread.

"If multiple threads enter the rule, and only one is to create the folder structure... how should we develop the rule thread-safely?"
Typically this would be very simple: Stop trying to manage transactions manually and rely on the retrying behaviour of Alfresco. As I already outlined in previous responses, multiple transactions may enter the rule and create the folder structure in parallel, but only one will be allowed to commit while all others will be rejected by the DB with update conflicts, causing the transactions to retry and then they will find the folder structure already created by the one transaction that succeeded.

"But "FileExistsException" is thrown for non first threads that arrive, and CMIS RTH does not contemplate it as retryable."
And that is correct - a FileExistsException is never retryable. If a file already exists with the same name that means there exists a node already committed to the DB, and there is no chance at all that simply retrying the current transactions will see the other node vanish (unless some user / code may be deleting it in a parallel transaction).

"NonRootNodeWithoutParentsException its an odd one and could understand it's due to some cache bug, but what about the second one; InvalidNodeRefException: Node does not exist?"

And that would be the correct exception when you create the folder structure in a nested transaction using a RTH, and then try to use that to move something in the outer transaction. Due to transaction isolation, the NodeRef does indeed not exist (yet) for the outer transaction, triggering an InvalidNodeRefException.

"FileFolderUtils.makeFolders, for instance, should work, as it states in the javadoc: checks for the presence of, and creates as necessary, the folder structure in the provided path"

Yes, but the JavaDoc does NOT mention that the lookup will be done with system privileges which means it can return existing folders that the user is not allowed to access and thus cause AccessDeniedException failures later on. Never take JavaDoc comments for 100% accurate without checking the implementation.

Re: Thread-safe action rules for massive concurrency (MySQL)

iru — Thu, 01 Jun 2017 10:46:47 GMT

Morning,

Some quick clarifications.

- Yep, it's clear to me the RTH involvement in the CMIS endpoint. Thanks for re-pointing it out

- Yep, you're right, maybe it's more a transaction issue, but from the client's point of view, it's a multiple-concurrent thread test, even if on the server side, as seen, it's leading to multiple-transactions.

- Related to the InvalidNodeRefException, the nodeRef ID the exception is pointing to its not of the just created folder, but of the just uploaded file.
To me, the problem is not in the just created folder, but in the fact that in an inner transaction snapshot, due to isolation, the just just to be uploaded node does not exist.
To reinforce my statement, this happens too when we have 1 RTH with both actions (create and move) within the same block of code.
Just for clarification, this happens in both our mysql and postgres setup.

- Yep, FileFolderUtils.makeFolders is done with system privileges, checked code besides javadoc.
However, no AccessDeniedException is raised in this tests and it's not what we're discussing, although thanks for re-pointing it out. What I was empathizing is how it's programmed, first check, then create if not existing (just what we want).

"If a file already exists with the same name that means there exists a node already committed to the DB"
That's what FileFolderUtils.makeFolders deals with;
1. first checks > node does not exists
2. then tries to create > node exists and gives the exception (created by another thread / transaction)

However, this method does throw "FileExistsException" internally, which, in first instance does not make sense if its job is to check first.

Guess database isolation again.
We do had some custom code in order to create the hierarchical structure of folders, using FileFolderService in a recursive check/creation loop, but wanted to simplify the test and that's why we opted for FileFolderUtils, thought it would be better a product provided solution than our own code.
__

Yesterday we tested it on an SQLServer based installation, and got other round of different exceptions, will update first post now.
__

To sum it up, we still don't know howto solve this within the rule implementation.
- Using 1 RTH for the folder creation, leads to different kind of exceptions, different depending on the database provider, like mysql and sqlserver (although not in postgres)
- Not using RTH, leads to some threads not to be aware of the "initizliation" of the folderization structure that only one of the batch will create.
When multiple threads (with multiple transactions) try to create the folders FileExistsException is thrown on all but one. As this is not retryable, n-1 errors are thrown to the client-side, being batch size n.
Relying on the RTH of the cmis endpoint seems not to be a valid solution, as the exception is not handled as a retryable one.

We will try to move part of the code to our custom API, hoping CMIS deals ok with it and hope we got some clarifying results.
__

Even if we seem to be in a vicious replying circle, insight appreciated @Axel, I'm sure your highlights will help someone else.
pd: Feel free to change any misleading word/concept that may disrupt other community members. If not you, some moderator should be able to do so.

Re: Thread/Transaction-safe action rules for massive concurrency

afaust — Thu, 01 Jun 2017 10:57:37 GMT

I am actually a moderator, so there should be no issues. Though I really don't want to go about editing any replies because it could be even more confusing.

Ok, the InvalidNodeRef error inside a nested transaction is of course an issue with the isolation / non-committed state.

Multiple threads in multiple transactions running through the makeFolders code should not run into FileExistsException (again, issue with isolation may be cause here) - there should be something along the line of DeadlockLoserException or ConcurrencyFailureException, which are retryable.

I wouldn't call the reply-cycle "vicious" - just long-running since we don't seem to come to a simple, clear-cut answer to what is going wrong.

Re: Thread/Transaction-safe action rules for massive concurrency

iblanco — Thu, 01 Jun 2017 11:34:23 GMT

Really interesting post.

We had exactly the same issues some time ago trying to do exactly the same thing. It was long time ago so I might be misleaded in some details.

During our multiple attempts to solve the issue I can remember that we reached a point were creating a folder returned a FileExistsException but at the same time we could not get and use the folder itself.

I thought that Alfresco always set READ_COMMITED isolation level to all connections so I said "that must be a non repeteable read" but I have just rechecked it now and the documentation states clearly that by default the databases default isolation level is used so in the case of Mysql this is "REPETEABLE_READ" and non repeteable reads could not happen. So now i'm a bit clueless about how this happened.

In this case retrying the transaction could in fact solve the issue because if the new file already exists once the transaction is retried the logic that checks the existence of the folder will see the folder and won't try to create it so the exception won't happen again.

But of course that is something very especific to the use case and a developer can't expect RTH to be able to "guess" what different execution path the logic will follower when retrying.

I think that we tried to catch the FilExistsException and wrap it in some kind of retryable transaction so that the RTH would just retry it, but I think that this path had other issues.

Maybe doing the whole operation in a RTH with a new transactionm catching the FileExistsException in your code and restarting the opeartion by yourself (again in a new transaction) will do the trick. But to be honest I think that we tried this and another ton of things and in the end we think that we just gave up.

Re: Thread/Transaction-safe action rules for massive concurrency

tfrith — Thu, 01 Jun 2017 16:53:07 GMT

We had a similar issue (although we weren't creating folders as fine-grained as to the minute).
We found that you'd get two threads that try to create the same folder because before commit they can't see each other's new folders.

We ended up having another process create the folders ahead of time.

As you mentioned, this can leave a lot of empty folders which would have to be cleaned up later.
Would this be so bad if a cron task did it fairly often?

What if you changed the approach a bit:
1. don't trigger your action on an inbound rule for every document
2. use a cron task to handle documents in batches
3. for each batch:
a. make a pass through all the documents in the batch to determine required folders (to cover all the creation dates)
b. create all the folders
c. then move all the documents

That way you should never have one thread trying to create the same folder as another thread.

You can experiment with optimal batch size and frequency for your needs.

Re: Thread/Transaction-safe action rules for massive concurrency

iru — Fri, 21 Jul 2017 11:31:13 GMT

Been a while, just wanted to update status.

We ended up developing the desired functionality in the client's Alfresco API wrapper, where all operations from different projects join together.
Just before the upload methods. the desired folder structure is created, and documents are directly uploaded to their destination.

Via opencmis library, no concurrency / thread-safe issues
However, solution is not generic enough so as to benefit from it
__

@iblanco thanks for the insight, seems we're all working on the same direction, stepping same rocks in the way. guess we require a more advanced knowledge / time, in order to provide a solution, compatible with current architecture design.

@tfrith, thanks for the idea, but one of our requirements is to make all the upload operation, up to the destination folder, synchronously, so no option for batch movements afterwards.

stay hungry!