Skip to content

HBASE-29669 Implement basic row cache#7901

Open
EungsopYoo wants to merge 1 commit intoapache:HBASE-29585from
EungsopYoo:HBASE-29669
Open

HBASE-29669 Implement basic row cache#7901
EungsopYoo wants to merge 1 commit intoapache:HBASE-29585from
EungsopYoo:HBASE-29669

Conversation

@EungsopYoo
Copy link
Contributor

No description provided.

@EungsopYoo
Copy link
Contributor Author

EungsopYoo commented Mar 11, 2026

@wchevreuil
I have completed the implementation of the basic row cache. Please review it.

By the way, it seems like CI is not working, do you know why?

@liuxiaocs7
Copy link
Contributor

Hi, @EungsopYoo

CI has been migrated from Jenkins to GitHub Actions, and the pre-commit GitHub Jenkins job should have been disabled, more details: https://lists.apache.org/thread/z6o5hhsd9goh5j7fcl4bnwzsktlwlwl4

Some previously created feature branches also have this issue; perhaps we could give it a try, such as: https://lists.apache.org/thread/hrl7dktkrb8zwwbv62my68lvtbplv7sr

Thanks!

Comment on lines +258 to +264

void removeTableLevelBarrier(HRegion region) {
regionLevelBarrierMap.computeIfPresent(region, (k, counter) -> {
int remaining = counter.decrementAndGet();
return (remaining <= 0) ? null : counter;
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this method should be renamed by removeRegionLevelBarrier?

Comment on lines +960 to +963
// For testing only
void setRowCache(RowCache rowCache) {
this.rowCache = rowCache;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

For testing only, we could use @RestrictedApi annotation?

private BlockCache l2Cache = null;
private MobFileCache mobFileCache;
private CacheStats cacheStats;
private final RowCache rowCache;
Copy link
Contributor

Choose a reason for hiding this comment

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

better move to L71 under MobFileCache?

Comment on lines +103 to +104
RSRpcServices rsRpcServices = this.regionServer.getRSRpcServices();
this.rowCache = rsRpcServices == null ? null : rsRpcServices.getServer().getRowCache();
Copy link
Contributor

Choose a reason for hiding this comment

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

better to use a method initRowCache() here, just like blockcache and mobfilecache does?

Comment on lines +2362 to +2363
// The row cache for the region has been enabled again
rowCache.removeTableLevelBarrier(region);
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto region level

Comment on lines +74 to +78
@Category({ RegionServerTests.class, MediumTests.class })
public class TestRowCache {
@ClassRule
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestRowCache.class);
Copy link
Contributor

@liuxiaocs7 liuxiaocs7 Mar 16, 2026

Choose a reason for hiding this comment

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

For new tests, it is best to use JUnit5 directly, since existing unit tests are currently being migrated to JUnit5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants