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

NSFS | Improve list objects performance on top of NS FS (PR 2/3) #7983

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

naveenpaul1
Copy link
Contributor

@naveenpaul1 naveenpaul1 commented Apr 17, 2024

Explain the changes

  1. This is Second PR for listing unordered objects.
  2. This PR contains changes in list_object(), TellDir value is passed to next list object result and same value is used to seekdir dir location.
  3. Global flag is added in config for controlling bucket list flow.
    NB : New parameter next_pos added to API list_objects to pass the seek position. I tried to pass this along with next_marker as a object but Ceph test failed when comparing next_marker.

Issues: Fixed #xxx / Gap #xxx

  1. Improve list objects performance on top of NS FS #6615

Testing Instructions:

  1. Run ListObject API after enabling global property ALLOW_NSFS_UNSORTED_OBJECTS_LIST, Result should retun with unsorted list.
  • Doc added/updated
  • Tests added

@guymguym guymguym modified the milestones: 5.15.4, 5.16.0 Apr 22, 2024
@naveenpaul1 naveenpaul1 force-pushed the nsfs_list_objects_2 branch 5 times, most recently from 5c0317a to f1ca72c Compare April 30, 2024 08:04
@naveenpaul1 naveenpaul1 marked this pull request as ready for review April 30, 2024 08:35
@guymguym guymguym modified the milestones: 5.16.z, 5.15.4 May 1, 2024
@romayalon romayalon modified the milestones: 5.15.4, 5.15.5 Jun 4, 2024
@naveenpaul1 naveenpaul1 force-pushed the nsfs_list_objects_2 branch 2 times, most recently from 770e1e5 to 48d2f71 Compare June 6, 2024 12:42
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/api/object_api.js Outdated Show resolved Hide resolved
config.js Outdated Show resolved Hide resolved
src/test/unit_tests/jest_tests/test_list_object.test.js Outdated Show resolved Hide resolved
delimiter: '/',
}, dummy_object_sdk);
expect(ls_obj_res.objects.map(it => it.key).length).toStrictEqual(keys_objects.length + 1);
//expect(ls_obj_res.objects.map(it => it.key)).toEqual(keys_objects);
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this test check? we should add more tests, specifically -

  1. 1000+ objects so we can check pagination.
    Pagination check should contain - multiple list_objects() calls, first one checks the value of the continuation token/next marker and then pass it to the next list_objects() call. and we need to make sure all keys return within these calls.
  2. Put objects in different heirarchies and do the same tests again.

src/test/unit_tests/jest_tests/test_list_object.test.js Outdated Show resolved Hide resolved
src/test/unit_tests/jest_tests/test_list_object.test.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/sdk/namespace_fs.js Outdated Show resolved Hide resolved
src/endpoint/s3/ops/s3_get_bucket.js Outdated Show resolved Hide resolved
@shirady
Copy link
Contributor

shirady commented Jun 9, 2024

Hi @naveenpaul1,
I found that when running the tests on MacOS some of them don't pass:
I tried: npx jest test_list_object.test.js
I see:

manage unsorted list objcts flow
Unsorted List objects
✓ List object unsorted with one object upload (12 ms)
✓ List object unsorted with multiple object upload (671 ms)
Telldir and Seekdir implementation
✓ telldir returns bigint (1 ms)
✓ seekdir expects bigint (13 ms)
✕ list dir files - telldir and seekdir. (2 ms)
✕ list dir files - Dir.read() and seekdir() (1 ms)
✕ list 10000 dir files - telldir and seekdir (1074 ms)

Details:

● manage unsorted list objcts flow › Telldir and Seekdir implementation › list dir files - telldir and seekdir.

expect(received).toBe(expected) // Object.is equality

Expected: 5
Received: 8

  134 |             }
  135 |             //total number of dir and files inside list_dir_root is 5
> 136 |             expect(total_dir_entries).toBe(total_files + 1);
      |                                       ^
  137 |         });
  138 |
  139 |         it('list dir files -  Dir.read() and seekdir()', async () => {

  at Object.toBe (src/test/unit_tests/jest_tests/test_list_object.test.js:136:39)

● manage unsorted list objcts flow › Telldir and Seekdir implementation › list dir files - Dir.read() and seekdir()

expect(received).toBe(expected) // Object.is equality

Expected: 0n
Received: 1n

  148 |                 const tell_dir = await dir_handle.telldir(DEFAULT_FS_CONFIG);
  149 |                 //verify tell_dir and dir_entry.off return same value
> 150 |                 expect(tell_dir).toBe(dir_entry.off);
      |                                  ^
  151 |                 dir_marker = {
  152 |                     dir_path: list_dir_root,
  153 |                     pos: dir_entry.off,

  at Object.toBe (src/test/unit_tests/jest_tests/test_list_object.test.js:150:34)

● manage unsorted list objcts flow › Telldir and Seekdir implementation › list 10000 dir files - telldir and seekdir

expect(received).toBe(expected) // Object.is equality

Expected: 10000
Received: 10501

  208 |             }
  209 |             //total number of dir and files inside list_dir_root is 5
> 210 |             expect(total_dir_entries).toBe(10000);
      |                                       ^
  211 |         }, 10000);
  212 |     });
  213 | });

  at Object.toBe (src/test/unit_tests/jest_tests/test_list_object.test.js:210:39)

@naveenpaul1
Copy link
Contributor Author

@shirady could you please try the test after running
npm run build:native

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

Successfully merging this pull request may close these issues.

None yet

4 participants