Skip to content

Commit

Permalink
Back out "Correctly record the rawBytesRead_ metric in TableScan" (#8536
Browse files Browse the repository at this point in the history
)

Summary:
Pull Request resolved: #8536

Original commit changeset: f56f72167ff4

Original Phabricator Diff: D52616884

The original change is causing negative value reported as `rawBytesRead` and failing queries: P1101501191

Reviewed By: mbasmanova

Differential Revision: D53089642

fbshipit-source-id: c2e9bd2acc987b870d555eaf9bd9d64d8a12fdfd
  • Loading branch information
Yuhta authored and facebook-github-bot committed Jan 25, 2024
1 parent e9f5424 commit 0f96c1c
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 9 deletions.
3 changes: 0 additions & 3 deletions velox/dwio/common/CacheInputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,6 @@ void CacheInputStream::loadPosition() {
// 'region_'
loadRegion.length = std::min<int32_t>(
loadQuantum_, region_.length - (loadRegion.offset - region_.offset));

// There is no need to update the metric in the loadData method because
// loadSync is always executed regardless and updates the metric.
loadSync(loadRegion);
}
auto* entry = pin_.checkedEntry();
Expand Down
1 change: 0 additions & 1 deletion velox/dwio/common/DirectBufferedInput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ std::vector<cache::CachePin> DirectCoalescedLoad::loadData(bool isPrefetch) {
}
input_->read(buffers, requests_[0].region.offset, LogType::FILE);
ioStats_->read().increment(size);
ioStats_->incRawBytesRead(size - overread);
ioStats_->incRawOverreadBytes(overread);
if (isPrefetch) {
ioStats_->prefetch().increment(size);
Expand Down
3 changes: 0 additions & 3 deletions velox/dwio/common/DirectInputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,6 @@ void DirectInputStream::loadPosition() {
loadedRegion_.length = (offsetInRegion_ + loadQuantum_ <= region_.length)
? loadQuantum_
: (region_.length - offsetInRegion_);

// Since the loadSync method updates the metric, but it is conditionally
// executed, we also need to update the metric in the loadData method.
loadSync();
}

Expand Down
3 changes: 1 addition & 2 deletions velox/exec/tests/TableScanTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,7 @@ TEST_F(TableScanTest, allColumns) {
auto scanNodeId = plan->id();
auto it = planStats.find(scanNodeId);
ASSERT_TRUE(it != planStats.end());
ASSERT_GT(it->second.peakMemoryBytes, 0);
ASSERT_GT(it->second.rawInputBytes, 0);
ASSERT_TRUE(it->second.peakMemoryBytes > 0);
EXPECT_LT(0, exec::TableScan::ioWaitNanos());
}

Expand Down

0 comments on commit 0f96c1c

Please sign in to comment.