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

Sometimes getMapper exception #2890

Open
uniquewings opened this issue Jun 20, 2023 · 11 comments
Open

Sometimes getMapper exception #2890

uniquewings opened this issue Jun 20, 2023 · 11 comments

Comments

@uniquewings
Copy link
Contributor

MyBatis version

3.5.9

Test case or example project

We have an application, it will start a grpc server, will load all mapper at main thread, and will receive business task from grpc client in grpc thread pool. We sometimes get the exception "Type xxx is not known to the MapperRegistry", but when we check the appliction log, we are confused why the exception will occur.

  1. first, we use sqlSessionTemplate.getConfiguration().addMapper(mapper); to add all the mappers. (main thread)
  2. then we use sqlSessionTemplate.getConfiguration().hasMapper(mapper); to check if the mapper add success.(main thread)
  3. last we will will wait mapper load finished by step1 and step 2, and then use sessionTemplate.getMapper(mapper); to get the mapper to use.(grpc thread pool)

As we check the code of MapperRegistry, when addMapper end, there's no way to remove mapper from the knownMappers. How did the exception occur when getMapper at step 3.

We expect getMapper success everytime at step 3.

@hazendaz
Copy link
Member

hazendaz commented Jun 20, 2023 via email

@uniquewings
Copy link
Contributor Author

Can you try latest? Typically we don't have resources to figure out old versions and if item was previously reported it could already be fixed. Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

________________________________ From: uniquewings @.> Sent: Tuesday, June 20, 2023 4:36:01 AM To: mybatis/mybatis-3 @.> Cc: Subscribed @.> Subject: [mybatis/mybatis-3] Sometimes getMapper exception (Issue #2890) MyBatis version 3.5.9 Test case or example project We have an application, it will start a grpc server, will load all mapper at main thread, and will receive business task from grpc client in grpc thread pool. We sometimes get the exception "Type xxx is not known to the MapperRegistry", but when we check the appliction log, we are confused why the exception will occur. 1. first, we use sqlSessionTemplate.getConfiguration().addMapper(mapper); to add all the mappers. (main thread) 2. then we use sqlSessionTemplate.getConfiguration().hasMapper(mapper); to check if the mapper add success.(main thread) 3. last we will will wait mapper load finished by step1 and step 2, and then use sessionTemplate.getMapper(mapper); to get the mapper to use.(grpc thread pool) As we check the code of MapperRegistry, when addMapper end, there's no way to remove mapper from the knownMappers. How did the exception occur when getMapper at step 3. We expect getMapper success everytime at step 3. — Reply to this email directly, view it on GitHub<#2890>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI5SB3AM7AHCOHUK64LXMFOHDANCNFSM6AAAAAAZM5UIOM. You are receiving this because you are subscribed to this thread.Message ID: @.>

We will not update to the latest version recently. I don't think it has already be fixed. The use of mybatis in our application has such details:

  1. we do not use xml define mapper, all mapper method define by annotation such as @select@delete.
  2. we do not use @Mapper annotation, we use @repository to mark mapper as bean and use addMapper to register mapper to mybatis when the bean init. And use hasMapper to check if add success.
  3. the business thread is grpc thread pool, not the main thread.

In application log, the hasMapper return true before the exception when call sessionTemplate.getMapper.

@harawata
Copy link
Member

Please try the latest version.
There was a possibly related fix #2709 .

@uniquewings
Copy link
Contributor Author

Please try the latest version. There was a possibly related fix #2709 .

#2709 seems like fix the race condition of Mapped Statements, but not fix the race condition of mapper. The knownMappers define in MapperRegistry has the same problem. When one thread addMapper for new mapper and reach resize of HashMap, another thread try to get a mapper that has been added already may return null.

public class MapperRegistry {
private final Configuration config;
private final Map<Class, MapperProxyFactory> knownMappers = new HashMap();
}

@uniquewings
Copy link
Contributor Author

Please try the latest version. There was a possibly related fix #2709 .

any progress?

@harawata
Copy link
Member

harawata commented Jul 5, 2023

@uniquewings ,

It is unclear whether you actually tried the latest version or not, but if the problem reproduces with the latest version, I have no idea why.

If the steps you posted are accurate, it is technically impossible, right?
So, there may be something you misunderstand about the thread management.

@uniquewings
Copy link
Contributor Author

@uniquewings ,

It is unclear whether you actually tried the latest version or not, but if the problem reproduces with the latest version, I have no idea why.

If the steps you posted are accurate, it is technically impossible, right? So, there may be something you misunderstand about the thread management.

Why it is technically impossible?

For example:

  1. We have 100 mappers, all the mappers are spring beans, and set lazy init.
  2. We have 5 business threads, all the mappers will only be initing when it's first using in business threads, and we will use configuration.addMapper(xxxMapper.class); in mapper's init period.
  3. Thread-1 init Test1Mapper and then use Test1Mapper to do business.
  4. After a while Thread-2 init Test2Mapper and the knownMappers in MapperRegistry reach resize when addMapper, there will have a short time that the knownMappers is not complete when resizing, if Thread-3 get Test1Mapper by sessionTemplate.getMapper(Test1Mapper.class) at that time, it will get the exception "Type Test1Mapper is not known to the MapperRegistry".

@harawata
Copy link
Member

harawata commented Jul 5, 2023

Ah, I'm sorry I didn't read your comment correctly.

So, your problem will be resolved if MyBatis uses ConcurrentHashMap for knownMappers, is that what you are saying?

@uniquewings
Copy link
Contributor Author

Yes, just like the #2709 use ConcurrentHashMap for StrictMap.

@hazendaz
Copy link
Member

hazendaz commented Jul 8, 2023

@uniquewings Is this something you can raise a pull request for?

@hazendaz
Copy link
Member

Hi, @uniquewings see my comment on the PR. I did merge it but not 100% that solves it for you. I don't see anything wrong with doing that and your situation sounds rather unique. If you could give it a try and let us know if that fixes it. I don't recall all the specifics on the original fix we had elsewhere but seemed like that extra bit was needed that you didn't add. I don't know if that is really necessary or not given the use case. Anyway, we publish to sonatype snapshots so you can pull there or just use your own build to try to see if that resolves it. Not sure when our next release will be but its probably sometime in near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants