Skip to content

HBASE-25249 Adding StoreContext #2800

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.

Sign up for GitHub

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

Merged
taklwu merged 10 commits into from
Jan 9, 2021
Merged

HBASE-25249 Adding StoreContext #2800

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
69f4aa2
HBASE-25249 Adding HStoreContext
abhishekkhanna01 Aug 4, 2020
2731227
fix checkstyle
Dec 22, 2020
baeef9c
additional fix checkstyle for HStoreContext
Dec 23, 2020
7725f5f
Address comments from stack
Dec 23, 2020
8a9af9f
make storecontext final in HStore and fix import
Dec 23, 2020
b3c9561
use storecontext with writer generation
Dec 23, 2020
5e87534
hide supplier from the getter of `FavoredNodes`
Dec 30, 2020
b2df475
address comments and remove the defaultFileContext from StoreContext
Jan 6, 2021
8a0269f
removed setter for encryptionContext in HFileContext and fixed indent…
Jan 6, 2021
4b8ab29
remove getters in constructor of HStore and remove getBlockSize from …
Jan 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ public Compression.Algorithm getCompression() {
return compressAlgo;
}

public void setCompression(Compression.Algorithm compressAlgo) {
this.compressAlgo = compressAlgo;
}

Copy link
Contributor

@saintstack saintstack Jan 5, 2021

Choose a reason for hiding this comment

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

Whats going on here?

HFileContext is supposed to be ' Read-only HFile Context Information' but here we are adding a setter. I see there are one or two setters but we also have HFileContextBuilder. Shouldn't we be going via the Builder making HFileContexts? And why is this method not added on the Builder? (Can it be added non-public to encourage users to go via the Builder)?

Copy link
Contributor Author

@taklwu taklwu Jan 6, 2021

Choose a reason for hiding this comment

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

fixed, and reverted back to builder pattern

Copy link
Contributor

@saintstack saintstack Jan 6, 2021

Choose a reason for hiding this comment

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

You haven't uploaded new PR so will leave this as unresovled for now.

Copy link
Contributor Author

@taklwu taklwu Jan 6, 2021

Choose a reason for hiding this comment

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

oops, I missed this change. and now it should have updated/removed.

public boolean isUseHBaseChecksum() {
return usesHBaseChecksum;
}
Expand Down
5 changes: 3 additions & 2 deletions hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/HFileOutputFormat2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import org.apache.hadoop.hbase.regionserver.BloomType;
import org.apache.hadoop.hbase.regionserver.HStore;
import org.apache.hadoop.hbase.regionserver.StoreFileWriter;
import org.apache.hadoop.hbase.regionserver.StoreUtils;
import org.apache.hadoop.hbase.util.BloomFilterUtil;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.CommonFSUtils;
Expand Down Expand Up @@ -369,8 +370,8 @@ private WriterLength getNewWriter(byte[] tableName, byte[] family, Configuration
encoding = encoding == null ? datablockEncodingMap.get(tableAndFamily) : encoding;
encoding = encoding == null ? DataBlockEncoding.NONE : encoding;
HFileContextBuilder contextBuilder = new HFileContextBuilder().withCompression(compression)
.withDataBlockEncoding(encoding).withChecksumType(HStore.getChecksumType(conf))
.withBytesPerCheckSum(HStore.getBytesPerChecksum(conf)).withBlockSize(blockSize)
.withDataBlockEncoding(encoding).withChecksumType(StoreUtils.getChecksumType(conf))
.withBytesPerCheckSum(StoreUtils.getBytesPerChecksum(conf)).withBlockSize(blockSize)
.withColumnFamily(family).withTableName(tableName);

if (HFile.getFormatVersion(conf) >= HFile.MIN_FORMAT_VERSION_WITH_TAGS) {
Expand Down
6 changes: 3 additions & 3 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@
import org.apache.hadoop.hbase.io.hfile.HFileContext;
import org.apache.hadoop.hbase.io.hfile.HFileContextBuilder;
import org.apache.hadoop.hbase.regionserver.BloomType;
import org.apache.hadoop.hbase.regionserver.HStore;
import org.apache.hadoop.hbase.regionserver.HStoreFile;
import org.apache.hadoop.hbase.regionserver.StoreFileWriter;
import org.apache.hadoop.hbase.regionserver.StoreUtils;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.hadoop.hbase.util.ChecksumType;
import org.apache.hadoop.hbase.util.CommonFSUtils;
Expand Down Expand Up @@ -540,8 +540,8 @@ public static StoreFileWriter createWriter(Configuration conf, FileSystem fs,
Compression.Algorithm compression, CacheConfig cacheConfig, Encryption.Context cryptoContext,
boolean isCompaction) throws IOException {
return createWriter(conf, fs, family, new Path(basePath, mobFileName.getFileName()),
maxKeyCount, compression, cacheConfig, cryptoContext, HStore.getChecksumType(conf),
HStore.getBytesPerChecksum(conf), family.getBlocksize(), BloomType.NONE, isCompaction);
maxKeyCount, compression, cacheConfig, cryptoContext, StoreUtils.getChecksumType(conf),
StoreUtils.getBytesPerChecksum(conf), family.getBlocksize(), BloomType.NONE, isCompaction);
}

/**
Expand Down
31 changes: 16 additions & 15 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HMobStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ public class HMobStore extends HStore {
private AtomicLong mobFlushedCellsSize = new AtomicLong();
private AtomicLong mobScanCellsCount = new AtomicLong();
private AtomicLong mobScanCellsSize = new AtomicLong();
private ColumnFamilyDescriptor family;
private Map<TableName, List<Path>> map = new ConcurrentHashMap<>();
private final IdLock keyLock = new IdLock();
// When we add a MOB reference cell to the HFile, we will add 2 tags along with it
Expand All @@ -107,16 +106,15 @@ public class HMobStore extends HStore {
public HMobStore(final HRegion region, final ColumnFamilyDescriptor family,
final Configuration confParam, boolean warmup) throws IOException {
super(region, family, confParam, warmup);
this.family = family;
this.mobFileCache = region.getMobFileCache();
this.homePath = MobUtils.getMobHome(conf);
this.mobFamilyPath = MobUtils.getMobFamilyPath(conf, this.getTableName(),
family.getNameAsString());
getColumnFamilyName());
List<Path> locations = new ArrayList<>(2);
locations.add(mobFamilyPath);
TableName tn = region.getTableDescriptor().getTableName();
locations.add(HFileArchiveUtil.getStoreArchivePath(conf, tn, MobUtils.getMobRegionInfo(tn)
.getEncodedName(), family.getNameAsString()));
.getEncodedName(), getColumnFamilyName()));
map.put(tn, locations);
List<Tag> tags = new ArrayList<>(2);
tags.add(MobConstants.MOB_REF_TAG);
Expand Down Expand Up @@ -209,7 +207,7 @@ public StoreFileWriter createWriterInTmp(String date, Path basePath, long maxKey
Compression.Algorithm compression, byte[] startKey,
boolean isCompaction) throws IOException {
MobFileName mobFileName = MobFileName.create(startKey, date, UUID.randomUUID()
.toString().replaceAll("-", ""), region.getRegionInfo().getEncodedName());
.toString().replaceAll("-", ""), getHRegion().getRegionInfo().getEncodedName());
return createWriterInTmp(mobFileName, basePath, maxKeyCount, compression, isCompaction);
}

Expand All @@ -226,9 +224,10 @@ public StoreFileWriter createWriterInTmp(String date, Path basePath, long maxKey
public StoreFileWriter createWriterInTmp(MobFileName mobFileName, Path basePath,
long maxKeyCount, Compression.Algorithm compression,
boolean isCompaction) throws IOException {
return MobUtils.createWriter(conf, region.getFilesystem(), family,
new Path(basePath, mobFileName.getFileName()), maxKeyCount, compression, cacheConf,
cryptoContext, checksumType, bytesPerChecksum, blocksize, BloomType.NONE, isCompaction);
return MobUtils.createWriter(conf, getFileSystem(), getColumnFamilyDescriptor(),
new Path(basePath, mobFileName.getFileName()), maxKeyCount, compression, getCacheConfig(),
getEncryptionContext(), StoreUtils.getChecksumType(conf),
StoreUtils.getBytesPerChecksum(conf), blocksize, BloomType.NONE, isCompaction);
}

/**
Expand All @@ -245,10 +244,10 @@ public void commitFile(final Path sourceFile, Path targetPath) throws IOExceptio
validateMobFile(sourceFile);
LOG.info(" FLUSH Renaming flushed file from {} to {}", sourceFile, dstPath);
Path parent = dstPath.getParent();
if (!region.getFilesystem().exists(parent)) {
region.getFilesystem().mkdirs(parent);
if (!getFileSystem().exists(parent)) {
getFileSystem().mkdirs(parent);
}
if (!region.getFilesystem().rename(sourceFile, dstPath)) {
if (!getFileSystem().rename(sourceFile, dstPath)) {
throw new IOException("Failed rename of " + sourceFile + " to " + dstPath);
}
}
Expand All @@ -261,7 +260,7 @@ public void commitFile(final Path sourceFile, Path targetPath) throws IOExceptio
private void validateMobFile(Path path) throws IOException {
HStoreFile storeFile = null;
try {
storeFile = new HStoreFile(region.getFilesystem(), path, conf, this.cacheConf,
storeFile = new HStoreFile(getFileSystem(), path, conf, getCacheConfig(),
BloomType.NONE, isPrimaryReplicaStore());
storeFile.initReader();
} catch (IOException e) {
Expand Down Expand Up @@ -352,9 +351,11 @@ public List<Path> getLocations(TableName tableName) throws IOException {
locations = map.get(tableName);
if (locations == null) {
locations = new ArrayList<>(2);
locations.add(MobUtils.getMobFamilyPath(conf, tableName, family.getNameAsString()));
locations.add(MobUtils.getMobFamilyPath(conf, tableName, getColumnFamilyDescriptor()
.getNameAsString()));
locations.add(HFileArchiveUtil.getStoreArchivePath(conf, tableName,
MobUtils.getMobRegionInfo(tableName).getEncodedName(), family.getNameAsString()));
MobUtils.getMobRegionInfo(tableName).getEncodedName(), getColumnFamilyDescriptor()
.getNameAsString()));
map.put(tableName, locations);
}
} finally {
Expand Down Expand Up @@ -388,7 +389,7 @@ private MobCell readCell(List<Path> locations, String fileName, Cell search,
MobFile file = null;
Path path = new Path(location, fileName);
try {
file = mobFileCache.openFile(fs, path, cacheConf);
file = mobFileCache.openFile(fs, path, getCacheConfig());
return readPt != -1 ? file.readCell(search, cacheMobBlocks, readPt)
: file.readCell(search, cacheMobBlocks);
} catch (IOException e) {
Expand Down
Loading