Opened 6 years ago

Last modified 6 years ago

#4648 assigned defect

Transaction synchronization problem

Reported by: ppawel@… Owned by: ppawel@…
Priority: major Milestone:
Component: osmosis Version:
Keywords: Cc: ppawel@…

Description

When running something like this:

osmosis -v 3 --read-xml-change ~/Downloads/999.osc --tee-change --write-changedb-change authFile=~/authFile --write-pgsql-change authFile=~/authFile

I get the exception below. The problem is that two transactions are open in the same thread and they should be committed in LIFO order, see:

https://jira.springsource.org/browse/SPR-7306

The patch I attached simply reverses the order of calling complete on change tee sinks.

Oct 24, 2012 9:19:58 PM org.openstreetmap.osmosis.core.Osmosis run
INFO: Pipeline executing, waiting for completion.
Oct 24, 2012 9:20:22 PM org.springframework.jdbc.core.JdbcTemplate? extractReturnedResults
INFO: Added default SqlReturnResultSet? parameter named #result-set-1
Oct 24, 2012 9:20:22 PM org.openstreetmap.osmosis.core.pipeline.common.ActiveTaskManager? waitForCompletion
SEVERE: Thread for task 1-read-xml-change failed
java.lang.IllegalStateException?: Cannot deactivate transaction synchronization - not active

at org.springframework.transaction.support.TransactionSynchronizationManager?.clearSynchronization(TransactionSynchronizationManager?.java:314)
at org.springframework.transaction.support.TransactionSynchronizationManager?.clear(TransactionSynchronizationManager?.java:449)
at org.springframework.transaction.support.AbstractPlatformTransactionManager?.cleanupAfterCompletion(AbstractPlatformTransactionManager?.java:1008)
at org.springframework.transaction.support.AbstractPlatformTransactionManager?.processCommit(AbstractPlatformTransactionManager?.java:804)
at org.springframework.transaction.support.AbstractPlatformTransactionManager?.commit(AbstractPlatformTransactionManager?.java:723)
at org.openstreetmap.osmosis.changedb.common.DatabaseContext?.commitTransaction(DatabaseContext?.java:96)
at org.openstreetmap.osmosis.changedb.v0_6.PostgreSqlChangeWriter?.complete(PostgreSqlChangeWriter?.java:96)
at org.openstreetmap.osmosis.core.tee.v0_6.ChangeTee?$ProxyChangeSinkChangeSource?.complete(ChangeTee?.java:151)
at org.openstreetmap.osmosis.core.tee.v0_6.ChangeTee?.complete(ChangeTee?.java:91)
at org.openstreetmap.osmosis.xml.v0_6.XmlChangeReader?.run(XmlChangeReader?.java:111)
at java.lang.Thread.run(Thread.java:722)

Oct 24, 2012 9:20:22 PM org.openstreetmap.osmosis.core.Osmosis main
SEVERE: Execution aborted.
org.openstreetmap.osmosis.core.OsmosisRuntimeException?: One or more tasks failed.

at org.openstreetmap.osmosis.core.pipeline.common.Pipeline.waitForCompletion(Pipeline.java:146)
at org.openstreetmap.osmosis.core.Osmosis.run(Osmosis.java:92)
at org.openstreetmap.osmosis.core.Osmosis.main(Osmosis.java:37)
at sun.reflect.NativeMethodAccessorImpl?.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl?.invoke(NativeMethodAccessorImpl?.java:57)
at sun.reflect.DelegatingMethodAccessorImpl?.invoke(DelegatingMethodAccessorImpl?.java:43)
at java.lang.reflect.Method.invoke(Method.java:601)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchStandard(Launcher.java:329)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:239)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:409)
at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:352)
at org.codehaus.classworlds.Launcher.main(Launcher.java:47)

Attachments (1)

osmosis.diff (1.0 KB) - added by ppawel@… 6 years ago.

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by ppawel@…

comment:1 Changed 6 years ago by ppawel@…

  • Cc ppawel@… added

comment:2 Changed 6 years ago by brett@…

  • Owner changed from brett@… to ppawel@…
  • Status changed from new to assigned

I'm not keen to apply this change. I think the real bug is that Spring
transactions are visible outside of the task that created them. A
transaction created by one task shouldn't have any impact on transactions
created by other tasks. On the other hand, having to detach and re-attach
transactions from the current thread every time the task is exited and
re-entered may be very inefficient.

At a minimum, the patch needs to be updated in a couple of ways:

  • The current comment points to a SpringSource? Jira ticket, but it doesn't

explain how this is relevant to Osmosis and how reversing the order results
in correct transaction handling where multiple independent tasks are
involved.

consistency.

comment:3 Changed 6 years ago by ppawel@…

The JIRA ticket is about having two transactions open in the same thread - as it happens with an Osmosis pipeline that has two database-related tasks in it.

So there are two transactions (A and B) open at the start of each task and then they are closed in the same order in ChangeTee#complete?(). That causes a problem and my patch fixes it.

Whether it is a correct solution - I'm not sure. IMO the proper fix would be refactoring the whole Osmosis database related tasks implementation so that it uses one transaction manager, one data source etc. because right now there is a lot of duplication of the code in different plugins and Spring components are also duplicated which goes rather directly against Spring philosophy.

In any case, I've changed my approach to implementing my plugin - I copied the whole pgsnapshot plugin and I'm embedding my code into it instead of trying to make it work in a separate task but one pipeline.

What it means is that I will be able to execute my processing logic in one simple step:

osmosis -v 3 --read-xml-change OSC_FILE --write-owldb-change authFile=...

So as you can see I no longer need the --change-tee task hence I don't have the transaction problem anymore.

I leave it up to you what should be done with this patch/ticket.

comment:4 Changed 6 years ago by brett@…

I understand how the Jira ticket is relevant, I just felt that the comment in the patch didn't adequately explain the issue in Osmosis terms. 6 months down the track I'll probably forget what we were talking about and wonder why reversing the order of commit helps :-) To be honest, I'm not sure why reversing the order of completion helps even now. Are the two transactions being joined somehow? Is the second transaction attaching itself to the first transaction? I'm hesitant to change anything until I understand exactly what's going on here.

All Osmosis tasks are supposed to operate independently. Having multiple tasks influencing each other deviates from this. You're quite right in saying that this is going against normal Spring usage. I'm only using Spring because it drastically reduces the amount of JDBC code required, it's not being used to manage the lifecycle of the entire pipeline (ie. it's not being used for dependency injection). I'm not sure if we'll ever get to a point where we can use a single transaction manager across tasks. Each task will always require its own data source (I believe). To support multiple datasources in a single transaction we'd possibly have to go down the path of using XA transactions that can support any number of underlying transactional resources, but that's not a decision I'd take lightly.

As an aside, there's a simple workaround to ensure multiple transactions don't influence each other. You can use a couple of --buffer-change tasks after the --change-tee which will give each transactional task its own thread to run in. If you do that they can both attach a Spring transaction to the thread without treading on each others toes.

It's probably a good idea to duplicate the existing pgsnapshot plugin, at least for now until you get it in a stable state. I try to avoid code duplication generally, but I have already duplicated a number of times in the existing codebase. It's a bit messy, but allowing each task implementation to evolve independently has worked well so far. Having said that, I'm definitely open to ideas on how to improve things.

comment:5 Changed 6 years ago by ppawel@…

I'm sorry I got a bit carried away with my "Spring philosophy" comment. Obviously you are right that having separation between plugins is a good thing as it is one of Osmosis' main strengths - ability to create a Unix-like pipeline where every part does its job and only its job.

For those reasons I also take back my comment about plugins using one transaction manager, data source etc. - it certainly does not make sense. I was just maybe a bit frustrated with code duplication between plugins but that's a different issue entirely and far easier to resolve with moderate refactoring.

As for the problem itself - one comment in this JIRA task states:

We currently have the general assumption that only one transaction manager may actively control transaction synchronization at any time. So when mixing different transaction managers, all but one need to have their "transactionSynchronization(Name)" property switched to SYNCHRONIZATION_NEVER. We'll revisit this in Spring 3.1.

It looks like this is what was happening when I try to put both pgsnapshot and changedb plugins in one pipeline - both of them had their own transaction managers.

As I mentioned, I went down the path of having a single plugin (owldb) and it is really working out - even performance-wise it's better because some database operations are not performed multiple times in the pipeline.

Last edited 6 years ago by ppawel@… (previous) (diff)
Note: See TracTickets for help on using tickets.