kreait / laravel-firebase

A Laravel package for the Firebase PHP Admin SDK
https://github.com/kreait/firebase-php
MIT License
995 stars 161 forks source link

Slow response times on GAE & Cloud Run #28

Closed martijnvdgrift closed 4 years ago

martijnvdgrift commented 4 years ago

Hey there!

I was wondering whether you guys could help me with an issue I'm facing. App.php:

$app->withFacades();
$app->register(Kreait\Laravel\Firebase\ServiceProvider::class);

DomainService.php

public static function insertMessage(String $customerId, Message $message)
    {
        $collectionDocument = app('firebase.firestore')
            ->database()
            ->collection(ChatService::$collectionName)
            ->document($customerId);

        return $collectionDocument
            ->collection(ChatService::$subCollectionName)
            ->newDocument()
            ->create($message->jsonSerialize());
    }

When I run this app on Google App Engine (Flex) or Google Cloud Run and invoke this method call via a Rest call, the average response time is around 16 seconds. When running locally, this is around 500ms.

Do you know where the increase in latency might come from? We've also used https://github.com/firevel/firestore/tree/master/src, which doesn't have this problem, but Firevel is missing other functionalities.

Versions used: "kreait/laravel-firebase": "^1.5", "laravel/lumen-framework": "^6.0"

jeromegamez commented 4 years ago

That's an interesting problem! 🤔

Are you using a Service Account JSON file locally and on GAE/GCR? If so, please try leaving the FIREBASE_CREDENTIALS environment variable empty on the Google Infrastructure - this should force the SDK to use auto-discovery and discover the GAE/GCR environment, which should save some of the expensive roundtrips via external internet connections.

I hope this helps!

martijnvdgrift commented 4 years ago

Hey, thanks for the quick reaction. On local dev, I'm exposing GOOGLE_APPLICATION_CREDENTIALS as Environment variable. On GAE/GCR, I'm not setting any env vars.

jeromegamez commented 4 years ago

Darn... I was hoping it would be as easy as that. For the moment I have no other ideas, but I will try it out tomorrow on GAE/GCR as well, if I can at least reproduce the issue, I can hopefully also figure out why this is happening. 🤞

constantijn commented 4 years ago

Did some digging and i found a fix. If i open up this file: https://github.com/kreait/gcp-metadata-php/blob/master/src/GcpMetadata.php

and replace the constructor with:

public function __construct()
{
    $this->client = new \GuzzleHttp\Client();
}

Instead of the auto injected guzzle client then performance returns to normal. 28 seconds down to 200 milliseconds.

As for WHY this is a problem i have no idea, but this does make the problem go away.

Is there anything I can to do help to get this fixed in an official release so I don't have to monkeypatch my php code? :)

Edit: I should probably add that we found that calls to the GCP meta data service were taking just under 5 seconds each before this fix, and after this they're back down to 2-3 milliseconds each.

jeromegamez commented 4 years ago

You already did! Thank you for digging into it and finding a solution!

As you're currently already monkeypatching 😅, could you please try modifying the Factory in https://github.com/kreait/firebase-php/blob/master/src/Firebase/Factory.php#L418

from

} elseif ((new GcpMetadata())->isAvailable()) {

to

} elseif ((new GcpMetadata(new Client()))->isAvailable()) {

and see if this yields the same improvements? If so, I'll release a patch release today and look for a more sustainable solution after that.


Some background: I initially planned to rely on Google's packages as less as possible and to make the SDK HTTP-Client-independent (as it's currently tied to Guzzle). Unfortunately, I never came around really doing it, and so, the SDK is currently in this weird state that I'm duplicating some of the functionality that google/auth already provides. I'm not happy with that at all, but I think now could be the time to finally embrace Guzzle and the google/auth package, at least for the 4.x releases 😅

constantijn commented 4 years ago

Oh, i came across that part of the code in my bug hunting ;)

That won't (completely) work because a few lines above that we call:

        $serviceAccount = $this->getServiceAccount();

That one ends up going into new Discovery\OnGoogleCloudPlatform(), which has the same metadata problem.

jeromegamez commented 4 years ago

Ah, of course! 🤦‍♂

Could you please add/replace

"kreait/firebase-php": "dev-discovery"

to your project's composer.json to see if this helps? It's not pretty, but if it works, I would consider it good enough ^^

constantijn commented 4 years ago

Ok, that causes issues:

1) The application is back to being slow

and 2)

If i run composer install it works just fine but if i run composer install --no-interaction --no-dev it crashes with:

(1/1) ErrorClass 'Kreait\Laravel\Firebase\ServiceProvider' not found

in Application.php line 196 at Application->register('Kreait\Laravel\Firebase\ServiceProvider')in app.php line 82

jeromegamez commented 4 years ago

Thanks for testing it out! I think the —no-dev issue could be resolved by changing the dependency to dev-discovery as 4.41.1 or by changing the minimum stability - but that‘s not the point, especially if it‘s back at being slow.

I‘ll start working on a better, more permanent solution and let you know once I have something ready. Thanks for your help and your patience!

constantijn commented 4 years ago

Thanks for the replies so far, and looking forward to being able to rip this ugly monkey patch out of my codebase ;)

jeromegamez commented 4 years ago

I just released a new version of kreait/gcp-metadata which should be available on Packagist by now as well.

Could you please do a composer update or add "kreait/gcp-metadata": "^1.1" to your composer.json - I hope this will resolve the issue, it's basically the same thing you already did 😅

constantijn commented 4 years ago

Seems to work :) Going to do some more testing but I ripped out my monkey patch and the response times on my rest calls are in the 100-200ms range where i expect them to be :)

constantijn commented 4 years ago

Looks like it works, thank you very much :)

jeromegamez commented 4 years ago

I'm glad, thanks for the feedback! 🌺