-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-24822 Add a command to support archive the earliest log file ma… #2202
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,8 +63,10 @@ | |
| import org.apache.hadoop.hbase.ipc.RpcServer; | ||
| import org.apache.hadoop.hbase.ipc.ServerCall; | ||
| import org.apache.hadoop.hbase.log.HBaseMarkers; | ||
| import org.apache.hadoop.hbase.regionserver.FlushLifeCycleTracker; | ||
| import org.apache.hadoop.hbase.regionserver.HRegion; | ||
| import org.apache.hadoop.hbase.regionserver.MultiVersionConcurrencyControl; | ||
| import org.apache.hadoop.hbase.regionserver.RegionServerServices; | ||
| import org.apache.hadoop.hbase.trace.TraceUtil; | ||
| import org.apache.hadoop.hbase.util.Bytes; | ||
| import org.apache.hadoop.hbase.util.CommonFSUtils; | ||
|
|
@@ -718,7 +720,7 @@ private void cleanOldLogs() throws IOException { | |
| if (logsToArchive != null) { | ||
| for (Pair<Path, Long> logAndSize : logsToArchive) { | ||
| this.totalLogSize.addAndGet(-logAndSize.getSecond()); | ||
| archiveLogFile(logAndSize.getFirst()); | ||
| moveLogFileToArchiveDir(logAndSize.getFirst()); | ||
| this.walFile2Props.remove(logAndSize.getFirst()); | ||
| } | ||
| } | ||
|
|
@@ -732,7 +734,7 @@ public static Path getWALArchivePath(Path archiveDir, Path p) { | |
| return new Path(archiveDir, p.getName()); | ||
| } | ||
|
|
||
| private void archiveLogFile(final Path p) throws IOException { | ||
| private void moveLogFileToArchiveDir(final Path p) throws IOException { | ||
| Path newPath = getWALArchivePath(this.walArchiveDir, p); | ||
| // Tell our listeners that a log is going to be archived. | ||
| if (!this.listeners.isEmpty()) { | ||
|
|
@@ -872,6 +874,33 @@ public Map<byte[], List<byte[]>> rollWriter(boolean force) throws IOException { | |
| } | ||
| } | ||
|
|
||
| public void archive(RegionServerServices services) throws IOException{ | ||
| if (getNumRolledLogFiles() < 1) { | ||
| return; | ||
| } | ||
| // get the earliest log of this WAL instance | ||
| Map.Entry<Path, WalProps> firstWALEntry = this.walFile2Props.firstEntry(); | ||
| // flush reigons if necessary | ||
| Map<byte[], List<byte[]>> regions = | ||
| this.sequenceIdAccounting.findLower(firstWALEntry.getValue().encodedName2HighestSequenceId); | ||
| if (regions != null) { | ||
| for (Map.Entry<byte[], List<byte[]>> entry : regions.entrySet()) { | ||
| String encodedRegionName = Bytes.toString(entry.getKey()); | ||
| HRegion r = (HRegion) services.getRegion(encodedRegionName); | ||
| if (r == null) { | ||
| LOG.warn("Failed to flush of {} when archive manually, because it is not online on us", | ||
| encodedRegionName); | ||
| return; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems the return value should not be void - otherwise the caller wouldn't know this scenario (region not found).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think of just continue, since the region is not online(maybe closed or moved), anyway we will not lost data.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When region close, the onRegionClose of SequenceIdAccounting should be called, so its regions will consist with onlineRegions in HRegionServer, if not, there must be something wrong, then the onlineRegions shall prevail.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or throw an Exception to notice client? |
||
| } | ||
| r.flushcache(entry.getValue(), false, FlushLifeCycleTracker.DUMMY); | ||
| } | ||
| } | ||
| // move the log file to archive dir | ||
| this.totalLogSize.addAndGet(-firstWALEntry.getValue().logSize); | ||
| moveLogFileToArchiveDir(firstWALEntry.getKey()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if IOException is thrown from this call ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, the move action should be called first. |
||
| this.walFile2Props.remove(firstWALEntry.getKey()); | ||
| } | ||
|
|
||
| // public only until class moves to o.a.h.h.wal | ||
| /** @return the size of log files in use */ | ||
| public long getLogFileSize() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be renamed to go with the updated method name ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix later. Thanks.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought again, it should be a common name if we want to improve it in future, such as archive a specify log file, archive all log files, etc.
WDYT? Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it make sense, i will remove the "earliest" from method name also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think ArchiveWALResponse should contain the status of archival (including error).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked the existing api in Admin.java, such as flushRegion, compactRegion, seems currently we dont use status, if there is something wrong at server side, we notice client by throw ServiceException.
Though the FlushRegionResponse has a flushed flag, but does not be used at client.
Maybe we should keep consistent?
Thanks for comment. @tedyu