rgw/s3select: json integration: identifying new data-source and execute a JSON flow. by galsalomon66 · Pull Request #49411 · ceph/ceph

galsalomon66 · 2022-12-13T14:33:16Z

integration of JSON flow
integration of SQL limit operator
scan range: it enables the user to select a range(start --> end) for SQL processing.
thus, the user has the flexibility to process a specific range within the object.
the significant use of the scan-range option is running parallel requests on the same object.
the user must "know" how to merge the return results from the parallel requests (Trino is one example). (https://trino.io/docs/current/connector/hive.html).
Trino alignments (1) upon pushing down the Trino-engine converts the SQL-statements(different from AWS), the s3select-parser handles that. (2) Trino rejects(HIVE_CURSOR_ERROR) some of s3select responds due to its size. RGW/S3select limits the size of responses.

s3select submodule

Signed-off-by: Albin Antony aantony@redhat.com
Signed-off-by: galsalomon66 gal.salomon@gmail.com

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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

cbodley · 2022-12-13T16:11:05Z

src/rgw/rgw_s3select.cc

+        s3select_syntax.get_error_description().c_str(),
+        s3select_resource_id);
+    ldpp_dout(this, 10) << "s3-select query: wrong json format; {" << s3select_syntax.get_error_description() << "}" << dendl;
+    return -1;


please use posix error codes like -EINVAL

cbodley · 2022-12-13T16:14:36Z

src/rgw/rgw_s3select.cc

+  if (s->obj_size == 0) {
+    status = run_s3select_on_json(m_sql_query.c_str(), nullptr, 0);
+  } else {
+    auto bl_len = bl.get_num_buffers();


status is unchecked here, then we immediately go on to call status = run_s3select_on_json(...) below

do we need this special case for obj_size == 0 at all? it should be fine to loop over an empty bl.buffers()

as for the obj_size == 0 i recall we needed a specific flow for CSV upon zero size object.
it will be re-check.

status is unchecked here, then we immediately go on to call status = run_s3select_on_json(...) below

do we need this special case for obj_size == 0 at all? it should be fine to loop over an empty bl.buffers()

upon empty-object(zero size), the bl = buffer-list is empty, thus buffer-list loop does not execute, no call to s3select-function, which results with a wrong response.

if (s->obj_size == 0) { ... } returns the appropriate empty response.

cbodley · 2022-12-13T16:16:09Z

src/rgw/rgw_s3select.cc

+      m_aws_response_handler.update_processed_size(it.length());
+      status = run_s3select_on_json(m_sql_query.c_str(), &(it)[0], it.length());
+      if(status<0) {
+        break;


shouldn't we return this error, instead of overwriting it with status = run_s3select_on_json(...) just below?

galsalomon66 · 2022-12-14T04:40:58Z

a good start with teuthology
8 of 8 had passed with success

http://pulpito.front.sepia.ceph.com/gsalomon-2022-12-13_20:26:47-rgw:verify-json_rgw_integration-distro-default-smithi/

galsalomon66 · 2022-12-20T13:14:07Z

8 of 8 had passed with success
http://pulpito.front.sepia.ceph.com/gsalomon-2022-12-20_12:06:46-rgw:verify-json_rgw_integration-distro-default-smithi/

galsalomon66 · 2022-12-21T02:36:01Z

1 dead
7 had passed with success
http://pulpito.front.sepia.ceph.com/gsalomon-2022-12-20_19:48:17-rgw:verify-json_rgw_integration-distro-default-smithi/

galsalomon66 · 2022-12-21T15:37:22Z

1 dead
7 had passed with success
http://pulpito.front.sepia.ceph.com/gsalomon-2022-12-21_14:00:17-rgw:verify-json_rgw_integration-distro-default-smithi/

galsalomon66 · 2022-12-23T05:13:48Z

1 dead
7 had passed with success
http://pulpito.front.sepia.ceph.com/gsalomon-2022-12-22_19:38:44-rgw:verify-json_rgw_integration-distro-default-smithi/

mattbenjamin · 2023-01-01T18:34:23Z

I don't think commit subject we intend to merge should have "WIP:" prefixed

galsalomon66 · 2023-01-01T18:36:06Z

sure. i will remove that.

…

On Sun, Jan 1, 2023 at 8:34 PM Matt Benjamin ***@***.***> wrote: I don't think commit subject we intend to merge should have "WIP:" prefixed — Reply to this email directly, view it on GitHub <#49411 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFYVMYTXRD4GHMDZNZL7EA3WQHE3XANCNFSM6AAAAAAS5JA7OU> . You are receiving this because you were assigned.Message ID: ***@***.***>

galsalomon66 · 2023-01-12T16:39:37Z

the last teuthology run had failed and pointed to valgrind issues.
on work.

galsalomon66 · 2023-01-23T14:20:15Z

http://pulpito.front.sepia.ceph.com/gsalomon-2023-01-23_13:06:56-rgw:verify-json_rgw_integration-distro-default-smithi/

7 had passed successfully
1 dead

galsalomon66 · 2023-01-24T10:49:06Z

http://pulpito.front.sepia.ceph.com/gsalomon-2023-01-24_09:10:46-rgw:verify-json_rgw_integration-distro-default-smithi/

all 8 jobs and s3select-tests had passed successfully.

galsalomon66 · 2023-01-29T17:30:30Z

http://pulpito.front.sepia.ceph.com/gsalomon-2023-01-29_15:19:28-rgw:verify-json_rgw_integration-distro-default-smithi/
all tests had failed (16), it seems related to selinux
there is no failure related to s3select
13 s3select s3-tests had passed with success

galsalomon66 · 2023-02-16T12:19:01Z

https://pulpito.ceph.com/gsalomon-2023-02-16_10:43:13-rgw:verify-json_rgw_integration-distro-default-smithi/
all 4 jobs had passed successfully

galsalomon66 · 2023-02-19T04:06:12Z

http://pulpito.front.sepia.ceph.com/gsalomon-2023-02-18_05:53:05-rgw:verify-json_rgw_integration-distro-default-smithi/
all 4 jobs had passed successfully

galsalomon66 · 2023-02-23T21:41:06Z

https://pulpito.ceph.com/gsalomon-2023-02-19_03:59:51-rgw:verify-json_rgw_integration-distro-default-smithi/

47 s3select tests had passed successfully

galsalomon66 · 2023-03-02T06:58:34Z

http://pulpito.front.sepia.ceph.com/gsalomon-2023-02-26_12:32:11-rgw:verify-json_rgw_integration-distro-default-smithi/
22 had passed with success
the rest of the jobs had failed, it does not seem related to s3select tests(no failures to s3select)

s3select submodule integrate the limit-operator into RGW. current changes is for CSV format integrate the limit-operator with CSV-flow, and JSON-flow. an update of s3select submodule a fix for the input-serialization-type selection debug functionality. fix for error handling adding the scan-range feature. i.e. enables the user to define the range of processing(text only) remove the temp variable(the s3select-layer handles the sql-result setting upon JSON flow) adding documentation. for JSON, SQL limit-operator, scan-range option. a fix for JSON test framework in s3select module investigate crash. replace len with it.length() editorial changes; non-initialized variables adding validation for offset&length. (it seems that the offset&length causes Invalid reads, in some cases) changes related to trino integration.(1) syntax issues related to trino-engine statement conbersions(s3select parser handles that) (2). (2). trino rejects(HIVE_CURSUR_ERROR) some of the s3select responses do to its size, the change is about control the size of the respond. update JSON documentation trino changes: response in paging (limitation on size), trino syntax issues. Signed-off-by: galsalomon66 <gal.salomon@gmail.com>

cbodley · 2023-03-02T22:47:14Z

i see that all of those failed jobs were using objectstore/filestore-xfs.yaml. filestore was removed recently in #49528; maybe you had rebased your ceph branch but not your suite-branch?

since rgw/verify looks clean, i think this just needs to run against the full rgw suite? once you rebase the suite-branch, the rgw suite at --subset 1/3 is only ~44 jobs in total

galsalomon66 · 2023-03-03T06:48:17Z

https://pulpito.ceph.com/gsalomon-2023-03-03_04:37:38-rgw:verify-json_rgw_integration-distro-default-smithi/
1 dead, all the rest had passed with success.
24 jobs were scheduled (no limit)

cbodley · 2023-03-03T12:48:14Z

https://pulpito.ceph.com/gsalomon-2023-03-03_04:37:38-rgw:verify-json_rgw_integration-distro-default-smithi/ 1 dead, all the rest had passed with success. 24 jobs were scheduled (no limit)

this is still just the rgw:verify subsuite. can you please run the rgw suite at --subset 1/3?

cbodley · 2023-03-03T19:05:45Z

passed qa in https://pulpito.ceph.com/gsalomon-2023-03-03_14:42:02-rgw-json_rgw_integration-distro-default-smithi/

cbodley · 2023-03-03T19:36:34Z

@galsalomon66 can you please make sure that a tag gets added to the s3select repo corresponding to the commit our submodule now points to?

galsalomon66 · 2023-03-05T14:25:49Z

@cbodley
s3select the latest tag
https://github.com/ceph/s3select/releases/tag/V0.5-trino

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com>

doc/radosgw: Redd up s3select.rst as followup to #49411 Reviewed-by: Zac Dover <zac.dover@proton.me>

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com> (cherry picked from commit 78b42ee)

…0384-to-reef reef: doc/radosgw: Redd up s3select.rst as followup to #49411 Reviewed-by: Cole Mitchell <cole.mitchell.ceph@gmail.com>

github-actions bot added the rgw label Dec 13, 2022

albin-antony approved these changes Dec 13, 2022

View reviewed changes

albin-antony mentioned this pull request Dec 13, 2022

rgw/s3select: json integration #49317

Closed

14 tasks

cbodley changed the title ~~json integration: identifying new data-source and execute a JSON flow.~~ rgw/s3select: json integration: identifying new data-source and execute a JSON flow. Dec 13, 2022

cbodley reviewed Dec 13, 2022

View reviewed changes

galsalomon66 assigned galsalomon66 and unassigned galsalomon66 Dec 14, 2022

galsalomon66 force-pushed the json_rgw_integration branch 2 times, most recently from a1825e0 to d0b40ef Compare January 1, 2023 18:29

galsalomon66 force-pushed the json_rgw_integration branch from 288eecd to bd9a64f Compare January 8, 2023 22:39

galsalomon66 requested a review from a team as a code owner January 10, 2023 08:40

github-actions bot added the documentation label Jan 10, 2023

galsalomon66 force-pushed the json_rgw_integration branch from 7a771ef to d5fe6d8 Compare January 10, 2023 13:47

galsalomon66 added the DNM label Jan 10, 2023

galsalomon66 force-pushed the json_rgw_integration branch from d5fe6d8 to 44a3446 Compare January 11, 2023 16:22

galsalomon66 force-pushed the json_rgw_integration branch 2 times, most recently from 1c32c2a to 8a531b5 Compare January 19, 2023 15:37

galsalomon66 force-pushed the json_rgw_integration branch from 5c2e294 to 92b2354 Compare January 27, 2023 15:37

galsalomon66 force-pushed the json_rgw_integration branch from fa57139 to ed9785c Compare February 8, 2023 04:39

galsalomon66 requested a review from a team as a code owner February 16, 2023 08:52

galsalomon66 force-pushed the json_rgw_integration branch from 76aa0ca to 506840d Compare February 23, 2023 21:45

galsalomon66 force-pushed the json_rgw_integration branch from dbd262b to 781ff4d Compare March 2, 2023 08:06

galsalomon66 force-pushed the json_rgw_integration branch from 781ff4d to 9f20e75 Compare March 2, 2023 22:28

cbodley approved these changes Mar 3, 2023

View reviewed changes

cbodley mentioned this pull request Mar 3, 2023

s3select: json query tests ceph/s3-tests#474

Closed

cbodley merged commit d08115f into main Mar 3, 2023

cbodley deleted the json_rgw_integration branch March 3, 2023 19:35

cbodley mentioned this pull request Mar 3, 2023

reef: json integration: identifying new data-source and execute json flow. #50374

Merged

anthonyeleven added a commit to anthonyeleven/ceph that referenced this pull request Mar 5, 2023

doc/radosgw: Redd up s3select.rst as followup to ceph#49411

768230c

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com>

anthonyeleven mentioned this pull request Mar 5, 2023

doc/radosgw: Redd up s3select.rst as followup to #49411 #50384

Merged

14 tasks

anthonyeleven added a commit to anthonyeleven/ceph that referenced this pull request Mar 5, 2023

doc/radosgw: Redd up s3select.rst as followup to ceph#49411

b1be592

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com>

anthonyeleven added a commit to anthonyeleven/ceph that referenced this pull request Mar 6, 2023

doc/radosgw: Redd up s3select.rst as followup to ceph#49411

78b42ee

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com>

zdover23 added a commit that referenced this pull request Mar 7, 2023

Merge pull request #50384 from anthonyeleven/anthonyeleven/tweak-50374

96019dd

doc/radosgw: Redd up s3select.rst as followup to #49411 Reviewed-by: Zac Dover <zac.dover@proton.me>

zdover23 pushed a commit to zdover23/ceph that referenced this pull request Mar 7, 2023

doc/radosgw: Redd up s3select.rst as followup to ceph#49411

8738e6d

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com> (cherry picked from commit 78b42ee)

zdover23 pushed a commit to zdover23/ceph that referenced this pull request Mar 7, 2023

doc/radosgw: Redd up s3select.rst as followup to ceph#49411

e201841

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com> (cherry picked from commit 78b42ee)

zdover23 pushed a commit to zdover23/ceph that referenced this pull request Mar 7, 2023

doc/radosgw: Redd up s3select.rst as followup to ceph#49411

259f091

Signed-off-by: Anthony D'Atri <anthonyeleven@users.noreply.github.com> (cherry picked from commit 78b42ee)

zdover23 added a commit that referenced this pull request Mar 11, 2023

Merge pull request #50414 from zdover23/wip-doc-2023-03-07-backport-5…

7cea374

…0384-to-reef reef: doc/radosgw: Redd up s3select.rst as followup to #49411 Reviewed-by: Cole Mitchell <cole.mitchell.ceph@gmail.com>

Conversation

galsalomon66 commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

cbodley Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

cbodley Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

galsalomon66 Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

galsalomon66 Dec 18, 2022

Choose a reason for hiding this comment

Uh oh!

cbodley Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

galsalomon66 commented Dec 14, 2022

Uh oh!

galsalomon66 commented Dec 20, 2022

Uh oh!

galsalomon66 commented Dec 21, 2022

Uh oh!

galsalomon66 commented Dec 21, 2022

Uh oh!

galsalomon66 commented Dec 23, 2022

Uh oh!

mattbenjamin commented Jan 1, 2023

Uh oh!

galsalomon66 commented Jan 1, 2023 via email

Uh oh!

galsalomon66 commented Jan 12, 2023

Uh oh!

galsalomon66 commented Jan 23, 2023

Uh oh!

galsalomon66 commented Jan 24, 2023

Uh oh!

galsalomon66 commented Jan 29, 2023

Uh oh!

galsalomon66 commented Feb 16, 2023

Uh oh!

galsalomon66 commented Feb 19, 2023

Uh oh!

galsalomon66 commented Feb 23, 2023

Uh oh!

galsalomon66 commented Mar 2, 2023

Uh oh!

cbodley commented Mar 2, 2023

Uh oh!

galsalomon66 commented Mar 3, 2023

Uh oh!

cbodley commented Mar 3, 2023

Uh oh!

cbodley commented Mar 3, 2023

Uh oh!

cbodley commented Mar 3, 2023

Uh oh!

galsalomon66 commented Mar 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

galsalomon66 commented Dec 13, 2022 •

edited

Loading