pusher / faros

Faros is a CRD based GitOps controller
Apache License 2.0
99 stars 15 forks source link

Faros takes a long time calculating the File.Log #123

Closed sebastianrosch closed 5 years ago

sebastianrosch commented 5 years ago

For us, Faros takes several minutes after checkout before it applies changes. After some investigation, I noticed that it calculates the File.Log using git blame. However, the log seems to never be used within Faros.

I understand that Faros uses Repo.GetAllFiles from https://github.com/pusher/git-store

What do you think about adding an option to not return the Log property, as it is not required here? Any thoughts on this?

JoelSpeed commented 5 years ago

Hi @sebastianroesch, we haven't seen this issue (yet?) but I understand why this could be frustrating. I've been looking at how we might be able to fix this without changing functionality too much.

I've been thinking about changing the git-store so that Blames are obtained on a per file basis and not by default (yes I appreciate this is a breaking change but I don't know how many people are using this library, mostly just pusher projects I assume), so there would maybe be a method git-store.File.GetLog() or git-store.Repo.LogFor(file git-store.File) and then this could hopefully optimise things a bit, WDYT?

I also noticed you guys have been testing with a fork of git-store, have you seen performance improvements?

sebastianrosch commented 5 years ago

Hey @JoelSpeed, in our fork we added a flag to skip generating the log, and it reduced the duration of a Reconcile from 4-5 minutes to < 10 seconds. https://github.com/slentzen-auth0/git-store/commit/09947afd7cb6cd1f27c41c74e9da0391f86a640e

Your proposed solution sounds cleaner, especially as the Log is not used at all in Faros.

JoelSpeed commented 5 years ago

Hey @JoelSpeed, in our fork we added a flag to skip generating the log, and it reduced the duration of a Reconcile from 4-5 minutes to < 10 seconds. slentzen-auth0/git-store@09947af

That's good to know! Definitely worth fixing then.

Your proposed solution sounds cleaner, especially as the Log is not used at all in Faros.

Indeed, though we do have some internal plans around adding some git based log information to the status of GitTrack and GitTrackObjects though whether this is on a file or branch/tag level we are undecided. Any thoughts on what kind of Git Information in status fields might be useful are very welcome!