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

Is sendEachForMulticast thread safe #950

Open
Laith-jalal opened this issue May 22, 2024 · 4 comments
Open

Is sendEachForMulticast thread safe #950

Laith-jalal opened this issue May 22, 2024 · 4 comments
Assignees

Comments

@Laith-jalal
Copy link

I was going through the sendEachForMulticast logic and didn't find anything to tell if it's thread safe or not, my current implementation is by calling the following in a new thread

FirebaseMessaging.getInstance().sendEachForMulticast(message)

Am I messing something here ? is my implementation thread safe ?

@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@emindeniz99
Copy link

The method should not have any thread issue, because to achieve high notification throughput, we need to call the method in multiple thread.

In production, we are using the predecessor "sendmulticast" method with 6+ thread to achieve 80k+ rpm on one java process. And we didn't face with any thread safety issue.

@meor9zcu12
Copy link

When using FCM admin SDK v9.3.0 sendEach api with 20 threads, 500 tokens per thread, we encountered OutOfMemoryError,

Caused by: java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
at java.base/java.lang.Thread.start0(Native Method)
at java.base/java.lang.Thread.start(Thread.java:809)
at java.base/java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:945)
at java.base/java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1364)
at com.google.common.util.concurrent.MoreExecutors$ListeningDecorator.execute(MoreExecutors.java:640)
at java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:145)
at com.google.common.util.concurrent.AbstractListeningExecutorService.submit(AbstractListeningExecutorService.java:79)
at com.google.firebase.FirebaseApp.submit(FirebaseApp.java:384)
at com.google.firebase.ImplFirebaseTrampolines.submitCallable(ImplFirebaseTrampolines.java:98)
at com.google.firebase.internal.CallableOperation.callAsync(CallableOperation.java:47)
at com.google.firebase.messaging.FirebaseMessaging$2.execute(FirebaseMessaging.java:227)
at com.google.firebase.messaging.FirebaseMessaging$2.execute(FirebaseMessaging.java:222)
at com.google.firebase.internal.CallableOperation.call(CallableOperation.java:36)
at com.google.firebase.messaging.FirebaseMessaging.sendEach(FirebaseMessaging.java:189)

@emindeniz99
Copy link

When using FCM admin SDK v9.3.0 sendEach api with 20 threads, 500 tokens per thread, we encountered OutOfMemoryError,

Caused by: java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
at java.base/java.lang.Thread.start0(Native Method)
at java.base/java.lang.Thread.start(Thread.java:809)
at java.base/java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:945)
at java.base/java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1364)
at com.google.common.util.concurrent.MoreExecutors$ListeningDecorator.execute(MoreExecutors.java:640)
at java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:145)
at com.google.common.util.concurrent.AbstractListeningExecutorService.submit(AbstractListeningExecutorService.java:79)
at com.google.firebase.FirebaseApp.submit(FirebaseApp.java:384)
at com.google.firebase.ImplFirebaseTrampolines.submitCallable(ImplFirebaseTrampolines.java:98)
at com.google.firebase.internal.CallableOperation.callAsync(CallableOperation.java:47)
at com.google.firebase.messaging.FirebaseMessaging$2.execute(FirebaseMessaging.java:227)
at com.google.firebase.messaging.FirebaseMessaging$2.execute(FirebaseMessaging.java:222)
at com.google.firebase.internal.CallableOperation.call(CallableOperation.java:36)
at com.google.firebase.messaging.FirebaseMessaging.sendEach(FirebaseMessaging.java:189)

It is normal :). sendEachForMulticast has performance issues. The method creates a thread and opens a connection per device token. You can use old API until 20 June or try to limit concurrency or device token count. Concurrent sending requests should not be high, depending on your hardware, RAM and network socket limits.
You can check the issue. #834 (comment)

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

7 participants