Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

storage/dotgit: search for incoming dir only once #935

Merged
merged 2 commits into from
Aug 29, 2018

Conversation

jfontan
Copy link
Contributor

@jfontan jfontan commented Aug 25, 2018

Search for incoming object directory was done once each time objects were accessed. This means a ReadDir of the objects path that is expensive. Now incoming directory is searched the first time an object is accessed and its name kept in DotGit to be reused.

This has the drawback that if the incoming object directory is created after the repo is opened it won't be found.

Related to #887

Search for incoming object directory was done once each time objects
were accessed. This means a ReadDir of the objects path that is
expensive. Now incoming directory is searched the first time an object
is accessed and its name kept in DotGit to be reused.

Signed-off-by: Javi Fontan <jfontan@gmail.com>
@jfontan jfontan requested review from smola, mcuadros and a team August 25, 2018 17:29
@jfontan
Copy link
Contributor Author

jfontan commented Aug 25, 2018

@noxora, can you check if this change is compatible with pre-commit hook?

@noxora
Copy link

noxora commented Aug 26, 2018

I do not see why this would not be compatible, it appears to be a solid change.

@smola
Copy link
Collaborator

smola commented Aug 27, 2018

I think new incoming directories can appear in subsequent calls?

@jfontan
Copy link
Contributor Author

jfontan commented Aug 27, 2018

@smola, yes, that can happen if this is the use case.

  • Open repository with go-git
  • Execute git pull that triggers pre hook
  • While the pre hook is running read from repository with go-git

While this is possible I don't think it's the common usage. Most probably go-git will be used inside the pre hook.

  • Execute git pull that triggers pre hook
  • pre hook executes a binary that uses go-git to read the repo

directoryContents, err := d.fs.ReadDir(objectsPath)
if err == nil {
for _, file := range directoryContents {
if strings.Split(file.Name(), "-")[0] == "incoming" && file.IsDir() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the same as strings.HasPrefix(file.Name(), "incoming")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

Also reformatted function comment and fixed some typos.

Signed-off-by: Javi Fontan <jfontan@gmail.com>
@noxora
Copy link

noxora commented Aug 27, 2018

@jfontan I thought similarly, it is possible that it could change, but if it is being used to write a hook, it shouldn't. However, if we were to treat this as volatile (which it could be, in some uncommon use cases) then this would be problematic.

@mcuadros mcuadros merged commit 5cc316b into src-d:master Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants