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.

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

rgw: add headers to guide cache update in 304 response #54587

Merged
merged 2 commits into from Jan 8, 2024

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Nov 21, 2023

revives the fix from #35284

needed an additional change to RGWGetObj::execute() to save the object attrs when read_op->prepare() returns an error (like -ERR_NOT_MODIFIED in this case)

Fixes: https://tracker.ceph.com/issues/45736

test case in ceph/s3-tests#533

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

IlsooByun and others added 2 commits November 24, 2023 10:35
when `ReadOp::prepare()` fails with `-ERR_NOT_MODIFIED`, it succeeded in
reading the head object for mtime and attrs like etag. make those attrs
available to RGWGetObj so we can still send ETag and other cache-related
response headers

Fixes: https://tracker.ceph.com/issues/45736

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
Copy link
Contributor Author

cbodley commented Jan 5, 2024

version_id = s->object->get_instance();
s->obj_size = s->object->get_obj_size();
attrs = s->object->get_attrs();
multipart_parts_count = read_op->params.parts_count;
if (op_ret < 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

what about other error codes? is it safe to get attributes, size etc, if the head read failed for some other reason?
maybe wrap with if (op_ret == 0 || op_ret == -ERR_NOT_MODIFIED) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the variables are safe to copy on a read error, they'd just have their default values

@yuvalif yuvalif self-requested a review January 8, 2024 17:45
@cbodley cbodley merged commit f872a2c into ceph:main Jan 8, 2024
10 of 11 checks passed
@cbodley
Copy link
Contributor Author

cbodley commented Jan 8, 2024

tyvm @yuvalif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants