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

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

Merged
mcuadros merged 2 commits intosrc-d:masterfrom
jfontan:improvement/cache-incoming-directory
Aug 29, 2018
Merged

storage/dotgit: search for incoming dir only once#935
mcuadros merged 2 commits intosrc-d:masterfrom
jfontan:improvement/cache-incoming-directory

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 a team, mcuadros and smola 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