') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); GH-34460: [C++][Parquet] Split arrow::FileReader::ReadRowGroups() for flexible async IO by Hor911 · Pull Request #34461 · apache/arrow · GitHub
Skip to content

GH-34460: [C++][Parquet] Split arrow::FileReader::ReadRowGroups() for flexible async IO - #34461

Closed
Hor911 wants to merge 2 commits into
apache:mainfrom
Hor911:flex_async_read_row_groups
Closed

GH-34460: [C++][Parquet] Split arrow::FileReader::ReadRowGroups() for flexible async IO#34461
Hor911 wants to merge 2 commits into
apache:mainfrom
Hor911:flex_async_read_row_groups

Conversation

@Hor911

@Hor911Hor911 commented Mar 5, 2023•
edited
Loading

Copy link
Copy Markdown

Rationale for this change

Current implementation of arrow::FileReader::ReadRowGroups() has sync interface. It complicates use in environments where additional threads are undesirable. Splitting this method into 2 parts will fix it. Details and usage examples are inside issue description.

What changes are included in this PR?

Two new methods in arrow::FileReader class

Are these changes tested?

Changes were tested actively in our private repository.

Are there any user-facing changes?

No changes to the current functionality. PR is simple enough and expects no regression.

@Hor911
Hor911 requested a review from wjones127 as a code ownerMarch 5, 2023 23:37
@github-actions

github-actionsBot commented Mar 5, 2023

Copy link
Copy Markdown

@github-actions

github-actionsBot commented Mar 5, 2023

Copy link
Copy Markdown

⚠️ GitHub issue #34460has been automatically assigned in GitHub to PR creator.

@koukou changed the title GH-34460: [C++] Split arrow::FileReader::ReadRowGroups() for flexible async IOGH-34460: [C++][Parquet] Split arrow::FileReader::ReadRowGroups() for flexible async IOMar 6, 2023
robot-piglet pushed a commit to ydb-platform/ydb that referenced this pull request Mar 6, 2023
@Hor911
[C++][Parquet] Split arrow::FileReader::ReadRowGroups() for flexible …
ff7e0c3
…async IO #34461apache/arrow#34461
wjones127
wjones127 reviewed Mar 6, 2023
View reviewed changes
Comment threadcpp/src/parquet/arrow/reader.h
Comment on lines +252 to +253
virtual ::arrow::Status WillNeedRowGroups(const std::vector<int>& row_groups,
const std::vector<int>& column_indices) = 0;

@wjones127wjones127Mar 6, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How does the caller know when the row groups are loaded? Should this return a Future instead?

@Hor911Hor911Mar 6, 2023

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It can't be expressed in this API. This method is translated into call of arrow::io::RandomAccessFile::WillNeed()

No-op is default and valid implementation of WillNeed. It means that no preload/prefetch is provided in this RAF implementation. All work will be done when ReadAt or ReadAsync is called.

Current Arrow API expect tight coupling between FileReader, ParquetFileReader and intermediate Cache. It is not possible to provide true async decoupling w/o significant API changes (it was discussed somewhere).

For my technique to work, one should provide special implementation of arrow::io::RandomAccessFile which will receive WillNeed, download the data and signals it in some "hidden" way. Not perfect, but possible to reach what I needed w/o API changes and any other side effects.

I think I'll be able to provide you link ro real use case tomorrow.

@westonpacewestonpaceMar 6, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

#14723 adds a filesystem method for "read many". I would like to see this method support plugging and splitting in the same way that ReadRangeCache does today (then, ReadRangeCache will only be needed if you need true "caching"). Then I think we can use that instead of the ReadRangeCache.

This will allow local filesystems to rely on the OS for plugging & splitting and will allow remote filesystems like S3 to adapt the algorithm to their needs. It's also async and returns a future reliably so you can then return a future from this method (I agree that would be desired).

@wgtmacwgtmacMar 7, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 for @westonpace's suggestion.

In addition, what if WillNeedRowGroups (w/ or w/o same inputs) has been called more than once? Maintaining the state is rather tricky according to my experience. If the new function only issues I/O hints to the RandomAccessFile, probably it is much easier to reason about the behavior directly from RandomAccessFile.

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Mar 6, 2023
@amol-

amol- commented Mar 30, 2023

Copy link
Copy Markdown
Member

Closing because it has been untouched for a while, in case it's still relevant feel free to reopen and move it forward 👍

@amol-amol- closed this Mar 30, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonpacewestonpacewestonpace left review comments
@wgtmacwgtmacwgtmac left review comments
@wjones127wjones127wjones127 left review comments

Assignees

No one assigned

Labels

awaiting changesAwaiting changesComponent: C++Component: Parquet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C++] Split arrow::FileReader::ReadRowGroups() to 2 methods for flexible async IO

5 participants

@Hor911@amol-@westonpace@wgtmac@wjones127