Skip to content

Add some documentation for CRT S3 Client#625

Open
azkrishpy wants to merge 1 commit into
mainfrom
doc-updates
Open

Add some documentation for CRT S3 Client#625
azkrishpy wants to merge 1 commit into
mainfrom
doc-updates

Conversation

@azkrishpy
Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.05%. Comparing base (a31a657) to head (2dbe076).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #625   +/-   ##
=======================================
  Coverage   89.05%   89.05%           
=======================================
  Files          23       23           
  Lines        7748     7748           
=======================================
  Hits         6900     6900           
  Misses        848      848           
Files with missing lines Coverage Δ
source/s3_client.c 91.27% <ø> (ø)
source/s3_meta_request.c 91.10% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@azkrishpy azkrishpy marked this pull request as ready for review April 24, 2026 23:12
Comment thread source/s3_client.c
#include <inttypes.h>
#include <math.h>

/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we have a design doc folder that in the past we intended as a place to put detailed description kinda docs. does that belong there?

Comment thread source/s3_client.c
*
* Threading model:
*
* All scheduling decisions run on a single event loop thread (process_work_event_loop),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

important point here: the way scheduler works is when it kicks off it moves all the pending work into separate area, i.e. it captures the state of the world and no one else is supposed to touch that state. then it does all the scheduling work and updates global state after.
its totally possible that more work will arrive while schedule is running, but it will not be touched until next iteration of scheduler

Comment thread source/s3_client.c
* Event loop groups:
*
* Bootstrap ELG (client_bootstrap->event_loop_group) drives networking. HTTP connections
* are pinned to loops in this group. One loop from this group is designated as the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pinned is a bit confusing in this case. you might want to expand on exactly what you mean

Comment thread source/s3_client.c
* The task runs s_s3_client_process_work_task(), which calls the vtable's process_work,
* defaulting to s_s3_client_process_work_default(). All scheduling decisions happen there.
*
* schedule_process_work_synced() is called from:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i feel conflicted about mentioning specific function names in doc. will be really hard to keep in sync with actual code

Comment thread source/s3_client.c
*
* A request moves through the pipeline as follows:
* meta_request->update() produces a request, which goes to s_acquire_mem_and_prepare_request()
* to reserve a buffer from the pool (may block if memory is full). Then vtable->prepare_request()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not block. it will just async wait until mem frees up

Comment thread source/s3_meta_request.c
* complete, it produces a CompleteMultipartUpload. When that finishes, update() returns
* work_remaining=false and the client removes the meta request.
*
* For GetObject, the state machine first sends a HeadObject or a ranged GET for part 1 to
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

its a lot more complicated than that. there is a doc on that in design folder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants