Skip to content
This repository was archived by the owner on Jul 7, 2022. It is now read-only.

Parallelize S3 pull/push files + various bug fixes.#10

Closed
didip wants to merge 37 commits intoblake-education:masterfrom
newrelic-forks:master
Closed

Parallelize S3 pull/push files + various bug fixes.#10
didip wants to merge 37 commits intoblake-education:masterfrom
newrelic-forks:master

Conversation

@didip
Copy link

@didip didip commented Oct 15, 2014

This is PR is essentially a merge of changes between 2 level of forks (aldrinleal and New Relic).

The changelog of what we, New Relic, made:

  • Use go fmt on all files.
  • Use the official fsouza/go-dockerclient
  • Set GOMAXPROCS to consume all available CPU.
  • Pulling files from S3 and pushing to S3 are parallelized using fixed pool of goroutines, currently hard-coded at 50.
  • Removed the ability to set S3 prefix path. It makes debugging image name difficult.
  • Logging functionality is now standarized to using fmt library.
  • Added various code cleanup and test fixes.

aldrinleal and others added 30 commits August 31, 2014 23:59
@aldrinleal
Code updated
24262c9
@aldrinleal
Fixing stuff
9a9470c
@aldrinleal
Fixing .gitignore
82911ff
@aldrinleal
Missing file
db013ca
@aldrinleal
Logging (duh) + Image Handling
311eac7
@aldrinleal
Other fixes
870ed67
@aldrinleal
Fixing
6b188e6
@aldrinleal
Adding env for Docker Endpoint
e983085
@aldrinleal
small troubleshooting
b814012
@aldrinleal
Cleanup / Adding Env Var
08e353c
@aldrinleal
Fixing endpoint
c5c76e2
@aldrinleal
Updating for go-dockerclient changes
be47384
custom go-dockerclient fork is no longer needed.
9e8bd83
renamed to didip’s fork of dogestry
01ea815
Fixed missing variables on logger.Info.
49b3d2b
Set maximum GOMAXPROCS
8222227
Tests are passing again.
1c8c0df
Finally multi thread putFiles
817bb4f
Removed S3 prefix path, it’s not very useful.
312c6aa
Do not hardcode shelled out path.
8681d77
Removed unused method: processPulled.
3c57c5c
Multithreaded getFiles.
af188a7
Control number of goroutines spawned during S3 push.
fadc56c 
We are using this pattern: 1 producer, N consumers(goroutines) where N is constant (atm it is 50).
Ignore io.EOF when pushing.
545811d
Remove the need of logger library. There’s 1 more place to do this.
c04a178
Remove the need to use custom logger library.
04ed86c
We really do not need this.
5812e98
small rename
9c3c9d0
Removed unnecessary writer.Close()
600a669
Removed unnecessary destFile.Close()
0df0a3d
relistan
relistan reviewed Oct 15, 2014
View reviewed changes
remote/s3_test.go
Copy link

@relistan relistan Oct 15, 2014

Choose a reason for hiding this comment

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

Doubt this can go upstream like this.

@relistan
Copy link

relistan commented Oct 15, 2014

There are a number of references in the PR to our fork. This is kind of annoying because we can't run them with blake-education instead of newrelic until this is merged. Maybe these can be fixed at merge time?

@didip didip mentioned this pull request Oct 16, 2014
Open
@didip
Copy link
Author

didip commented Oct 16, 2014

Closed and replaced with PR #11

@didip didip closed this Oct 16, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@didip @relistan @aldrinleal