Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refine S3 bucket usage #61

Closed
johnml1135 opened this issue Aug 26, 2023 · 1 comment
Closed

Refine S3 bucket usage #61

johnml1135 opened this issue Aug 26, 2023 · 1 comment
Assignees
@Enkidu93
Labels
refactor
Milestone
1.1 Mother Tongue...

Comments

@johnml1135
Copy link
Collaborator

johnml1135 commented Aug 26, 2023 •
edited
Loading

A collection of comments

  • When the stream buffer is opened, it uses 100MB. 3 are opened at the same time per ClearML job while uploading files. This should be ok - but with 8 going at the same time - that is 2.4 GB - a bit much. Can we:
    • Have "WriteFilesAsync" be only opened once at time?
    • Should we use the low level implementation?
    • Throw more memory at it - now at 2.5 GB.
@johnml1135 johnml1135 added this to the 1.1 Mother Tongue MVP milestone Aug 26, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New in Serval Aug 26, 2023
@Enkidu93 Enkidu93 moved this from 🆕 New to 🏗 In progress in Serval Aug 30, 2023
Enkidu93 added a commit that referenced this issue Sep 7, 2023
@Enkidu93
Reverted to low-level implementation (removing need for large buffers…
3177461 
…); fixes Refine S3 bucket usage #61

Also removed synchronous write; fixes S3 remove sync write (async only) #64

Note: Dispose has async calls/logic because "await using" on the StreamWriters in ClearMLNMTBuildJob calls DisposeAsync on the StreamWriters but Dispose on the BaseStream. This is the only way I could find to circumvent this - other solutions are welcome given there's a workaround I'm missing.

--ECL
@Enkidu93 Enkidu93 mentioned this issue Sep 7, 2023
Merged
@Enkidu93 Enkidu93 moved this from 🏗 In progress to 👀 In review in Serval Sep 8, 2023
johnml1135 pushed a commit that referenced this issue Sep 8, 2023
@Enkidu93 @johnml1135
Reverted to low-level implementation (removing need for large buffers…
cf57098 
…); fixes Refine S3 bucket usage #61

Also removed synchronous write; fixes S3 remove sync write (async only) #64

Note: Dispose has async calls/logic because "await using" on the StreamWriters in ClearMLNMTBuildJob calls DisposeAsync on the StreamWriters but Dispose on the BaseStream. This is the only way I could find to circumvent this - other solutions are welcome given there's a workaround I'm missing.
johnml1135 pushed a commit that referenced this issue Sep 8, 2023
@Enkidu93 @johnml1135
Reverted to low-level implementation (removing need for large buffers…
da821a7 
…); fixes Refine S3 bucket usage #61

Also removed synchronous write; fixes S3 remove sync write (async only) #64

Note: Dispose has async calls/logic because "await using" on the StreamWriters in ClearMLNMTBuildJob calls DisposeAsync on the StreamWriters but Dispose on the BaseStream. This is the only way I could find to circumvent this - other solutions are welcome given there's a workaround I'm missing.
@Enkidu93
Copy link
Collaborator

Enkidu93 commented Sep 8, 2023

Fixed in #80

@Enkidu93 Enkidu93 closed this as completed Sep 8, 2023
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Serval Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Enkidu93 Enkidu93

Labels
refactor
Projects
Serval
Status: ✅ Done
Milestone
1.1 Mother Tongue MVP
Development

No branches or pull requests
2 participants
@johnml1135 @Enkidu93