Skip to content
New issue

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

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

Already on GitHub? # to your account

Reduce the number of API calls performed by DatasetFSProvider.readFile() #3278

Closed
Tracked by #3250
benjamin-t-santos opened this issue Oct 25, 2024 · 4 comments · Fixed by #3279
Closed
Tracked by #3250

Reduce the number of API calls performed by DatasetFSProvider.readFile() #3278

benjamin-t-santos opened this issue Oct 25, 2024 · 4 comments · Fixed by #3279
Labels
bug Something isn't working priority-medium Not functioning - next quarter if capacity permits Technical Debt Includes Architecture, Code, Testing, Automation debt

Comments

@benjamin-t-santos
Copy link
Contributor

benjamin-t-santos commented Oct 25, 2024

Is your feature request related to a problem? Please describe.

Extenders can use vscode.workspace.fs.readFile(dsUri) to fetch a dataset's contents. It will also create an entry for the dataset/member in the FileSystemProvider if it does not already exist.

Currently, readFile() makes two MVS API calls. The first call is in remoteLookupForResource() (it calls fetchDataset() which calls allMembers() or dataSet() depending on the provided URI). The second call is in fetchDatasetAtUri() (calls getContents()). I believe we only need to make one API call with getContents().

Describe the solution you'd like

Rework fetchDatasetAtUri() to only make one API call by doing the following:

  1. Call getContents()
  2. If the API call is successful, update the entry with the contents (create a new entry if one does not already exist) and return it.
  3. If getContents() fails, the requested resource does not exist on remote and we returnnull.

That is a rough approach we could take. I have prototyped this in my own fork and there is a performance improvement, which is quite relevant when reading the contents of 100s or 1000s of members. I tested reading 600 members in parallel and with this optimization, it is twice as fast.

Describe alternatives you've considered

Using the DatasetFSProvider, I do not see a way to get the contents of a dataset that does not already have an entry without making two API calls.

Additional context

Here is a PR prototyping that I described above: #3279. I hope the team will consider this change and I am open to feedback.

Copy link

Thank you for raising this enhancement request.
The community has 90 days to vote on it.
If the enhancement receives at least 10 upvotes, it is added to our development backlog.
If it receives fewer votes, the issue is closed.

@JTonda JTonda added the priority-medium Not functioning - next quarter if capacity permits label Oct 29, 2024
@JTonda JTonda moved this from New Issues to In Progress in Zowe Explorer for VS Code Oct 29, 2024
@traeok traeok linked a pull request Oct 31, 2024 that will close this issue
15 tasks
@traeok traeok closed this as completed Oct 31, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Closed in Zowe Explorer for VS Code Oct 31, 2024
@JillieBeanSim JillieBeanSim added bug Something isn't working Technical Debt Includes Architecture, Code, Testing, Automation debt and removed enhancement New feature or request labels Nov 8, 2024
Copy link

github-actions bot commented Nov 8, 2024

Thank you for creating a bug report.
We will investigate the bug and evaluate its impact on the product.
If you haven't already, please ensure you have provided steps to reproduce the bug and as much context as possible.

@JillieBeanSim
Copy link
Contributor

changing label as this is a bug for extenders resulting in a regression from v2 to v3

@traeok
Copy link
Member

traeok commented Nov 8, 2024

I'm not sure how this would be considered a v2 -> v3 regression as this functionality was not available in v2, and this type of filesystem exposure was only introduced with v3. That said I do agree that it could be interpreted as a bug from the initial implementation (especially if it means that we can get it out with the 3.0.3 release).

@zFernand0 zFernand0 mentioned this issue Nov 14, 2024
15 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working priority-medium Not functioning - next quarter if capacity permits Technical Debt Includes Architecture, Code, Testing, Automation debt
Projects
Status: Closed
Development

Successfully merging a pull request may close this issue.

4 participants