virajjasani commented on code in PR #5119:
URL: https://github.com/apache/hbase/pull/5119#discussion_r1142821115
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AsyncFSWAL.java:
##########
@@ -778,32 +774,35 @@ private void waitForSafePoint() {
}
}
- protected final long closeWriter(AsyncWriter writer, Path path) {
- if (writer != null) {
- inflightWALClosures.put(path.getName(), writer);
- long fileLength = writer.getLength();
- closeExecutor.execute(() -> {
- try {
- writer.close();
- } catch (IOException e) {
- LOG.warn("close old writer failed", e);
- } finally {
- inflightWALClosures.remove(path.getName());
- }
- });
- return fileLength;
- } else {
- return 0L;
- }
+ private void closeWriter(AsyncWriter writer, Path path) {
+ inflightWALClosures.put(path.getName(), writer);
+ closeExecutor.execute(() -> {
+ try {
+ writer.close();
+ } catch (IOException e) {
+ LOG.warn("close old writer failed", e);
+ } finally {
+ // call this even if the above close fails, as there is no other
chance we can set closed to
+ // true, it will not cause big problems.
+ markClosedAndClean(path);
+ inflightWALClosures.remove(path.getName());
+ }
+ });
}
@Override
protected void doReplaceWriter(Path oldPath, Path newPath, AsyncWriter
nextWriter)
throws IOException {
Preconditions.checkNotNull(nextWriter);
waitForSafePoint();
- long oldFileLen = closeWriter(this.writer, oldPath);
- logRollAndSetupWalProps(oldPath, newPath, oldFileLen);
+ if (writer != null) {
+ long oldFileLen = writer.getLength();
+ logRollAndSetupWalProps(oldPath, newPath, oldFileLen);
+ closeWriter(writer, oldPath);
+ } else {
+ logRollAndSetupWalProps(oldPath, newPath, 0);
Review Comment:
Ah, we can come here after shutdown?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java:
##########
@@ -298,27 +298,34 @@
final Comparator<Path> LOG_NAME_COMPARATOR =
(o1, o2) -> Long.compare(getFileNumFromFileName(o1),
getFileNumFromFileName(o2));
- private static final class WalProps {
+ private static final class WALProps {
/**
* Map the encoded region name to the highest sequence id.
* <p/>
* Contains all the regions it has an entry for.
*/
- public final Map<byte[], Long> encodedName2HighestSequenceId;
+ final Map<byte[], Long> encodedName2HighestSequenceId;
/**
* The log file size. Notice that the size may not be accurate if we do
asynchronous close in
* sub classes.
*/
- public final long logSize;
+ final long logSize;
/**
* The nanoTime of the log rolling, used to determine the time interval
that has passed since.
*/
- public final long rollTimeNs;
+ final long rollTimeNs;
Review Comment:
Shall we also make them `private`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
