Skip to content
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.

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

HBASE-24474 Rename LocalRegion to MasterRegion #1811

Merged
merged 1 commit into from
Jun 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@
import org.apache.hadoop.hbase.master.procedure.ReopenTableRegionsProcedure;
import org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure;
import org.apache.hadoop.hbase.master.procedure.TruncateTableProcedure;
import org.apache.hadoop.hbase.master.region.MasterRegion;
import org.apache.hadoop.hbase.master.region.MasterRegionFactory;
import org.apache.hadoop.hbase.master.replication.AbstractPeerProcedure;
import org.apache.hadoop.hbase.master.replication.AddPeerProcedure;
import org.apache.hadoop.hbase.master.replication.DisablePeerProcedure;
Expand All @@ -144,7 +146,6 @@
import org.apache.hadoop.hbase.master.replication.UpdatePeerConfigProcedure;
import org.apache.hadoop.hbase.master.slowlog.SlowLogMasterService;
import org.apache.hadoop.hbase.master.snapshot.SnapshotManager;
import org.apache.hadoop.hbase.master.store.LocalStore;
import org.apache.hadoop.hbase.master.zksyncer.MasterAddressSyncer;
import org.apache.hadoop.hbase.master.zksyncer.MetaLocationSyncer;
import org.apache.hadoop.hbase.mob.MobFileCleanerChore;
Expand Down Expand Up @@ -450,7 +451,7 @@ public void run() {
private ProcedureStore procedureStore;

// the master local storage to store procedure data, etc.
private LocalStore localStore;
private MasterRegion masterRegion;
Copy link
Member

Choose a reason for hiding this comment

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

Good.


// handle table states
private TableStateManager tableStateManager;
Expand Down Expand Up @@ -964,8 +965,8 @@ private void finishActiveMasterInitialization(MonitoredTask status) throws IOExc
this.splitWALManager = new SplitWALManager(this);
}

// initialize local store
localStore = LocalStore.create(this);
// initialize master local region
masterRegion = MasterRegionFactory.create(this);
createProcedureExecutor();
Map<Class<?>, List<Procedure<MasterProcedureEnv>>> procsByType =
procedureExecutor.getActiveProceduresNoCopy().stream()
Expand Down Expand Up @@ -1543,8 +1544,8 @@ protected void stopServiceThreads() {

stopProcedureExecutor();

if (localStore != null) {
localStore.close(isAborted());
if (masterRegion != null) {
masterRegion.close(isAborted());
}
if (this.walManager != null) {
this.walManager.stop();
Expand All @@ -1563,7 +1564,7 @@ protected void stopServiceThreads() {
private void createProcedureExecutor() throws IOException {
MasterProcedureEnv procEnv = new MasterProcedureEnv(this);
procedureStore =
new RegionProcedureStore(this, localStore, new MasterProcedureEnv.FsUtilsLeaseRecovery(this));
new RegionProcedureStore(this, masterRegion, new MasterProcedureEnv.FsUtilsLeaseRecovery(this));
Copy link
Member

Choose a reason for hiding this comment

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

Having everything use a single region has me a little nervous. Seems like it'll make it easy for two unrelated subsystems to step on each others' toes later on -- conflicting row keys, columns, &c. This should be fine for initial work though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is to use different families. There is a known risk that, if someone stores a lot data in one of the families, it will slow down the start up of the whole HMaster, even if it is not necessary. We should document this in our ref guide. Can be a follow on issue?

procedureStore.registerListener(new ProcedureStoreListener() {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import org.apache.hadoop.hbase.Stoppable;
import org.apache.hadoop.hbase.conf.ConfigurationObserver;
import org.apache.hadoop.hbase.io.HFileLink;
import org.apache.hadoop.hbase.master.store.LocalStore;
import org.apache.hadoop.hbase.master.region.MasterRegionFactory;
import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
import org.apache.hadoop.hbase.util.StealJobQueue;
import org.apache.yetus.audience.InterfaceAudience;
Expand Down Expand Up @@ -161,7 +161,7 @@ public HFileCleaner(String name, int period, Stoppable stopper, Configuration co
protected boolean validate(Path file) {
return HFileLink.isBackReferencesDir(file) || HFileLink.isBackReferencesDir(file.getParent()) ||
StoreFileInfo.validateStoreFileName(file.getName()) ||
file.getName().endsWith(LocalStore.ARCHIVED_HFILE_SUFFIX);
file.getName().endsWith(MasterRegionFactory.ARCHIVED_HFILE_SUFFIX);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import org.apache.hadoop.hbase.Stoppable;
import org.apache.hadoop.hbase.conf.ConfigurationObserver;
import org.apache.hadoop.hbase.master.procedure.MasterProcedureUtil;
import org.apache.hadoop.hbase.master.store.LocalStore;
import org.apache.hadoop.hbase.master.region.MasterRegionFactory;
import org.apache.hadoop.hbase.wal.AbstractFSWALProvider;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
Expand Down Expand Up @@ -89,7 +89,7 @@ public LogCleaner(final int period, final Stoppable stopper, Configuration conf,
protected boolean validate(Path file) {
return AbstractFSWALProvider.validateWALFilename(file.getName()) ||
MasterProcedureUtil.validateProcedureWALFilename(file.getName()) ||
file.getName().endsWith(LocalStore.ARCHIVED_WAL_SUFFIX);
file.getName().endsWith(MasterRegionFactory.ARCHIVED_WAL_SUFFIX);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
import org.apache.hadoop.hbase.master.store.LocalStore;
import org.apache.hadoop.hbase.master.region.MasterRegionFactory;
import org.apache.yetus.audience.InterfaceAudience;

/**
Expand All @@ -42,7 +42,7 @@ protected long getTtlMs(Configuration conf) {

@Override
protected boolean valiateFilename(Path file) {
return file.getName().endsWith(LocalStore.ARCHIVED_HFILE_SUFFIX);
return file.getName().endsWith(MasterRegionFactory.ARCHIVED_HFILE_SUFFIX);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
import org.apache.hadoop.hbase.master.store.LocalStore;
import org.apache.hadoop.hbase.master.region.MasterRegionFactory;
import org.apache.yetus.audience.InterfaceAudience;

/**
Expand All @@ -42,6 +42,6 @@ protected long getTtlMs(Configuration conf) {

@Override
protected boolean valiateFilename(Path file) {
return file.getName().endsWith(LocalStore.ARCHIVED_WAL_SUFFIX);
return file.getName().endsWith(MasterRegionFactory.ARCHIVED_WAL_SUFFIX);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.hbase.master.store;
package org.apache.hadoop.hbase.master.region;

import static org.apache.hadoop.hbase.HConstants.HREGION_LOGDIR_NAME;

Expand Down Expand Up @@ -54,7 +54,7 @@
import org.apache.hbase.thirdparty.com.google.common.math.IntMath;

/**
* A region that stores data in a separated directory.
* A region that stores data in a separated directory, which can be used to store master local data.
* <p/>
* FileSystem layout:
*
Expand All @@ -79,14 +79,14 @@
* Notice that, you can use different root file system and WAL file system. Then the above directory
* will be on two file systems, the root file system will have the data directory while the WAL
* filesystem will have the WALs directory. The archived HFile will be moved to the global HFile
* archived directory with the {@link LocalRegionParams#archivedWalSuffix()} suffix. The archived
* archived directory with the {@link MasterRegionParams#archivedWalSuffix()} suffix. The archived
* WAL will be moved to the global WAL archived directory with the
* {@link LocalRegionParams#archivedHFileSuffix()} suffix.
* {@link MasterRegionParams#archivedHFileSuffix()} suffix.
*/
@InterfaceAudience.Private
public final class LocalRegion {
public final class MasterRegion {
Copy link
Member

Choose a reason for hiding this comment

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

We still need the wrapper class with delegation? How about have the factory manage creation of the HRegion (wiring up the wal, &c) and HMaster hold the instance of HRegion directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intentional. You can see the implementation of MasterRegion.update, we have to call flusherAndCompactor.onUpdate(); after each update. So if we expose the HRegion directly, the callers have to do this by their own, and I believe it will be easy to forget and then cause big problem...


private static final Logger LOG = LoggerFactory.getLogger(LocalRegion.class);
private static final Logger LOG = LoggerFactory.getLogger(MasterRegion.class);

private static final String REPLAY_EDITS_DIR = "recovered.wals";

Expand All @@ -100,12 +100,12 @@ public final class LocalRegion {
final HRegion region;

@VisibleForTesting
final LocalRegionFlusherAndCompactor flusherAndCompactor;
final MasterRegionFlusherAndCompactor flusherAndCompactor;

private LocalRegionWALRoller walRoller;
private MasterRegionWALRoller walRoller;

private LocalRegion(HRegion region, WALFactory walFactory,
LocalRegionFlusherAndCompactor flusherAndCompactor, LocalRegionWALRoller walRoller) {
private MasterRegion(HRegion region, WALFactory walFactory,
MasterRegionFlusherAndCompactor flusherAndCompactor, MasterRegionWALRoller walRoller) {
this.region = region;
this.walFactory = walFactory;
this.flusherAndCompactor = flusherAndCompactor;
Expand All @@ -128,7 +128,7 @@ private void shutdownWAL() {
}
}

public void update(UpdateLocalRegion action) throws IOException {
public void update(UpdateMasterRegion action) throws IOException {
action.update(region);
flusherAndCompactor.onUpdate();
}
Expand All @@ -142,17 +142,17 @@ public RegionScanner getScanner(Scan scan) throws IOException {
}

@VisibleForTesting
FlushResult flush(boolean force) throws IOException {
public FlushResult flush(boolean force) throws IOException {
return region.flush(force);
}

@VisibleForTesting
void requestRollAll() {
public void requestRollAll() {
walRoller.requestRollAll();
}

@VisibleForTesting
void waitUntilWalRollFinished() throws InterruptedException {
public void waitUntilWalRollFinished() throws InterruptedException {
walRoller.waitUntilWalRollFinished();
}

Expand All @@ -176,7 +176,7 @@ public void close(boolean abort) {
}
}

private static WAL createWAL(WALFactory walFactory, LocalRegionWALRoller walRoller,
private static WAL createWAL(WALFactory walFactory, MasterRegionWALRoller walRoller,
String serverName, FileSystem walFs, Path walRootDir, RegionInfo regionInfo)
throws IOException {
String logName = AbstractFSWALProvider.getWALDirectoryName(serverName);
Expand All @@ -197,7 +197,7 @@ private static WAL createWAL(WALFactory walFactory, LocalRegionWALRoller walRoll

private static HRegion bootstrap(Configuration conf, TableDescriptor td, FileSystem fs,
Path rootDir, FileSystem walFs, Path walRootDir, WALFactory walFactory,
LocalRegionWALRoller walRoller, String serverName) throws IOException {
MasterRegionWALRoller walRoller, String serverName) throws IOException {
TableName tn = td.getTableName();
RegionInfo regionInfo = RegionInfoBuilder.newBuilder(tn).setRegionId(REGION_ID).build();
Path tmpTableDir = CommonFSUtils.getTableDir(rootDir,
Expand All @@ -215,7 +215,7 @@ private static HRegion bootstrap(Configuration conf, TableDescriptor td, FileSys
}

private static HRegion open(Configuration conf, TableDescriptor td, FileSystem fs, Path rootDir,
FileSystem walFs, Path walRootDir, WALFactory walFactory, LocalRegionWALRoller walRoller,
FileSystem walFs, Path walRootDir, WALFactory walFactory, MasterRegionWALRoller walRoller,
String serverName) throws IOException {
Path tableDir = CommonFSUtils.getTableDir(rootDir, td.getTableName());
Path regionDir =
Expand Down Expand Up @@ -269,7 +269,7 @@ private static HRegion open(Configuration conf, TableDescriptor td, FileSystem f
return HRegion.openHRegionFromTableDir(conf, fs, tableDir, regionInfo, td, wal, null, null);
}

public static LocalRegion create(LocalRegionParams params) throws IOException {
public static MasterRegion create(MasterRegionParams params) throws IOException {
TableDescriptor td = params.tableDescriptor();
LOG.info("Create or load local region for table " + td);
Server server = params.server();
Expand All @@ -284,18 +284,21 @@ public static LocalRegion create(LocalRegionParams params) throws IOException {
Configuration conf = new Configuration(baseConf);
CommonFSUtils.setRootDir(conf, rootDir);
CommonFSUtils.setWALRootDir(conf, walRootDir);
LocalRegionFlusherAndCompactor.setupConf(conf, params.flushSize(), params.flushPerChanges(),
MasterRegionFlusherAndCompactor.setupConf(conf, params.flushSize(), params.flushPerChanges(),
params.flushIntervalMs());
conf.setInt(AbstractFSWAL.MAX_LOGS, params.maxWals());
if (params.useHsync() != null) {
conf.setBoolean(HRegion.WAL_HSYNC_CONF_KEY, params.useHsync());
}
if (params.useMetaCellComparator() != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add this in here?

Copy link
Contributor

Choose a reason for hiding this comment

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

This was missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For used as procedure store, this is not necessary. This is for preparing for storing root table in the future, as for root table, the row key is the same with meta table, and we need to use a special comparator. I think it is fine to change the comparator in the future?

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

conf.setBoolean(HRegion.USE_META_CELL_COMPARATOR, params.useMetaCellComparator());
}
conf.setInt(AbstractFSWAL.RING_BUFFER_SLOT_COUNT,
IntMath.ceilingPowerOfTwo(params.ringBufferSlotCount()));

LocalRegionWALRoller walRoller = LocalRegionWALRoller.create(td.getTableName() + "-WAL-Roller",
conf, server, walFs, walRootDir, globalWALRootDir, params.archivedWalSuffix(),
params.rollPeriodMs(), params.flushSize());
MasterRegionWALRoller walRoller = MasterRegionWALRoller.create(
td.getTableName() + "-WAL-Roller", conf, server, walFs, walRootDir, globalWALRootDir,
params.archivedWalSuffix(), params.rollPeriodMs(), params.flushSize());
walRoller.start();

WALFactory walFactory = new WALFactory(conf, server.getServerName().toString(), false);
Expand All @@ -311,7 +314,7 @@ public static LocalRegion create(LocalRegionParams params) throws IOException {
server.getServerName().toString());
}
Path globalArchiveDir = HFileArchiveUtil.getArchivePath(baseConf);
LocalRegionFlusherAndCompactor flusherAndCompactor = new LocalRegionFlusherAndCompactor(conf,
MasterRegionFlusherAndCompactor flusherAndCompactor = new MasterRegionFlusherAndCompactor(conf,
server, region, params.flushSize(), params.flushPerChanges(), params.flushIntervalMs(),
params.compactMin(), globalArchiveDir, params.archivedHFileSuffix());
walRoller.setFlusherAndCompactor(flusherAndCompactor);
Expand All @@ -320,6 +323,6 @@ public static LocalRegion create(LocalRegionParams params) throws IOException {
LOG.warn("Failed to create archive directory {}. Usually this should not happen but it will" +
" be created again when we actually archive the hfiles later, so continue", archiveDir);
}
return new LocalRegion(region, walFactory, flusherAndCompactor, walRoller);
return new MasterRegion(region, walFactory, flusherAndCompactor, walRoller);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,24 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.hbase.master.store;
package org.apache.hadoop.hbase.master.region;

import java.io.IOException;
import java.util.concurrent.TimeUnit;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Server;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
import org.apache.hadoop.hbase.client.Get;
import org.apache.hadoop.hbase.client.Result;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.regionserver.HRegion.FlushResult;
import org.apache.hadoop.hbase.regionserver.RegionScanner;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.yetus.audience.InterfaceAudience;

import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;

/**
* Used for storing data at master side. The data will be stored in a {@link LocalRegion}.
* The factory class for creating a {@link MasterRegion}.
*/
@InterfaceAudience.Private
public final class LocalStore {
public final class MasterRegionFactory {

// Use the character $ to let the log cleaner know that this is not the normal wal file.
public static final String ARCHIVED_WAL_SUFFIX = "$masterlocalwal$";
Expand Down Expand Up @@ -89,45 +82,8 @@ public final class LocalStore {
private static final TableDescriptor TABLE_DESC = TableDescriptorBuilder.newBuilder(TABLE_NAME)
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(PROC_FAMILY)).build();

private final LocalRegion region;

private LocalStore(LocalRegion region) {
this.region = region;
}

public void update(UpdateLocalRegion action) throws IOException {
region.update(action);
}

public Result get(Get get) throws IOException {
return region.get(get);
}

public RegionScanner getScanner(Scan scan) throws IOException {
return region.getScanner(scan);
}

public void close(boolean abort) {
region.close(abort);
}

@VisibleForTesting
public FlushResult flush(boolean force) throws IOException {
return region.flush(force);
}

@VisibleForTesting
public void requestRollAll() {
region.requestRollAll();
}

@VisibleForTesting
public void waitUntilWalRollFinished() throws InterruptedException {
region.waitUntilWalRollFinished();
}

public static LocalStore create(Server server) throws IOException {
LocalRegionParams params = new LocalRegionParams().server(server)
Copy link
Contributor

Choose a reason for hiding this comment

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

How comes we can drop these methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will return MasterRegion directly after this change, so we do not need these methods any more, users can use the methods in MasterRegion directly.

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

public static MasterRegion create(Server server) throws IOException {
MasterRegionParams params = new MasterRegionParams().server(server)
.regionDirName(MASTER_STORE_DIR).tableDescriptor(TABLE_DESC);
Configuration conf = server.getConfiguration();
long flushSize = conf.getLong(FLUSH_SIZE_KEY, DEFAULT_FLUSH_SIZE);
Expand All @@ -145,7 +101,6 @@ public static LocalStore create(Server server) throws IOException {
long rollPeriodMs = conf.getLong(ROLL_PERIOD_MS_KEY, DEFAULT_ROLL_PERIOD_MS);
params.rollPeriodMs(rollPeriodMs).archivedWalSuffix(ARCHIVED_WAL_SUFFIX)
.archivedHFileSuffix(ARCHIVED_HFILE_SUFFIX);
LocalRegion region = LocalRegion.create(params);
return new LocalStore(region);
return MasterRegion.create(params);
}
}
Loading