You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/10/31 19:22:23 UTC
[PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snasphot [ozone]
swamirishi opened a new pull request, #9230:
URL: https://github.com/apache/ozone/pull/9230
## What changes were proposed in this pull request?
RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snasphot. For this compactionJobId needs to be tracked and needs to be only warned if the snapshot was present on compaction begin.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13863
## How was this patch tested?
Added unit test
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #9230:
URL: https://github.com/apache/ozone/pull/9230
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482724799
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
Let me ask it differently -- when seeing this WARN message,
- is it a bug in the code?
- does it mean that some data (snapshot/diff/sst) is corrupted?
> ... However we want to be in a position to debug some other corner case scenario later on ...
Why not printing it in DEBUG?
The WARN message frequently shows up in OM log and it looks like some kind of data corruption.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2492401318
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
Suggestions
- call remove first and check the return value.
- Use info instead of warn
- Print also the jobInfo.
```java
final CompactionFileInfo removed = inflightCompactions.remove(inputFile);
if (removed == null) {
LOG.info("Input file not found in inflightCompactions: {}, compactionJobInfo is {}", inputFile, ...);
}
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482688388
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -493,14 +493,7 @@ public void onCompactionCompleted(RocksDB db,
builder = new CompactionLogEntry.Builder(trxId,
System.currentTimeMillis(),
inputFileCompactions.keySet().stream()
- .map(inputFile -> {
- if (!inflightCompactions.containsKey(inputFile)) {
- LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
- "compactionBeginListener.",
- inputFile);
- }
- return inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile));
- })
+ .map(inputFile -> inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile)))
Review Comment:
yeah let me use compute
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482688388
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -493,14 +493,7 @@ public void onCompactionCompleted(RocksDB db,
builder = new CompactionLogEntry.Builder(trxId,
System.currentTimeMillis(),
inputFileCompactions.keySet().stream()
- .map(inputFile -> {
- if (!inflightCompactions.containsKey(inputFile)) {
- LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
- "compactionBeginListener.",
- inputFile);
- }
- return inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile));
- })
+ .map(inputFile -> inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile)))
Review Comment:
yeah let me use compute
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495409554
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -493,14 +493,8 @@ public void onCompactionCompleted(RocksDB db,
builder = new CompactionLogEntry.Builder(trxId,
System.currentTimeMillis(),
inputFileCompactions.keySet().stream()
- .map(inputFile -> {
- if (!inflightCompactions.containsKey(inputFile)) {
- LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
- "compactionBeginListener.",
- inputFile);
- }
- return inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile));
- })
+ .map(inputFile -> inputFileCompactions.containsKey(inputFile) ?
+ inflightCompactions.get(inputFile) : inputFileCompactions.get(inputFile))
Review Comment:
@swamirishi , The logic is incorrect -- if inputFileCompactions contains inputFile, shouldn't get it from inputFileCompactions?
Also, why double accessing the map (containsKey and then get)?
```java
.map(inputFile -> {
final CompactionFileInfo f = inflightCompactions.get(inputFile);
return f != null ? f : inputFileCompactions.get(inputFile);
})
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495837212
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,7 +525,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
- inflightCompactions.remove(inputFile);
+ CompactionFileInfo removed = inflightCompactions.remove(inputFile);
+ if (removed == null) {
+ LOG.info("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.", inputFile);
Review Comment:
When you say show code/doc do you mean I should comment the code here?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482709661
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
Ideally I wouldn't have wanted to build this complex DAG to track all the compactions at the first place and would have wanted to just build a linked list of all the L0 sst flushes between 2 snapshots by tracking just the sst file flush. This would have made our entire Backup SstFileManagement and diff very straightforward
https://github.com/facebook/rocksdb/blob/36ce2e2a0a4d229d1805766b0e9534b1f708929f/java/src/test/java/org/rocksdb/EventListenerTest.java#L46-L58
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495798138
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -493,14 +493,8 @@ public void onCompactionCompleted(RocksDB db,
builder = new CompactionLogEntry.Builder(trxId,
System.currentTimeMillis(),
inputFileCompactions.keySet().stream()
- .map(inputFile -> {
- if (!inflightCompactions.containsKey(inputFile)) {
- LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
- "compactionBeginListener.",
- inputFile);
- }
- return inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile));
- })
+ .map(inputFile -> inputFileCompactions.containsKey(inputFile) ?
+ inflightCompactions.get(inputFile) : inputFileCompactions.get(inputFile))
Review Comment:
yeah you are right @szetszwo thank you for pointing it out. I have updated it
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
done
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495920539
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,7 +525,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
- inflightCompactions.remove(inputFile);
+ CompactionFileInfo removed = inflightCompactions.remove(inputFile);
+ if (removed == null) {
+ LOG.info("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.", inputFile);
Review Comment:
done added some comments
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2492586297
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
yeah sure let me make the chang
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495869815
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,7 +525,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
- inflightCompactions.remove(inputFile);
+ CompactionFileInfo removed = inflightCompactions.remove(inputFile);
+ if (removed == null) {
+ LOG.info("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.", inputFile);
Review Comment:
I mean -- Could you show the existing code/doc mentioning `compactionBeginListener`? Not adding new comments.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2483138511
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
Could you answer these two simple questions?
- is it a bug in the code?
- does it mean that some data (snapshot/diff/sst) is corrupted?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2492394288
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
@swamirishi , Thanks for explanation.
```diff
@@ -530,6 +524,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
+ }
inflightCompactions.remove(inputFile);
}
}
```
If (!inflightCompactions.containsKey(inputFile)), how to inflightCompactions.remove(inputFile)?
Sorry that I still don't know how to understand this change.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2491835783
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
After this change this log shouldn't occur. This warn log is going to be an indication of some unexpected behaviour in rocksdb compaction. No this is not a bug in the ozone code. It should not lead to any corruption. This is just a situation where there could be some possible race condition in the way rocksdb compaction.
What this means is that the DAG would have a compaction entry in memory DAG while the sst file is already deleted. This dag is only going to be used by snapshot diff operation. In case we are not able to find the file the snapshot diff would just fallback to full diff logic which can also happen in the case when the dag gets pruned. The only downside to this is that we would have to do a full diff which would be computationally expensive for the snapshots falling in this path of compaction
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9230:
URL: https://github.com/apache/ozone/pull/9230#issuecomment-3498654724
@szetszwo thank you for reviewing the patch
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482631351
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -493,14 +493,7 @@ public void onCompactionCompleted(RocksDB db,
builder = new CompactionLogEntry.Builder(trxId,
System.currentTimeMillis(),
inputFileCompactions.keySet().stream()
- .map(inputFile -> {
- if (!inflightCompactions.containsKey(inputFile)) {
- LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
- "compactionBeginListener.",
- inputFile);
- }
- return inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile));
- })
+ .map(inputFile -> inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile)))
Review Comment:
This is inefficient -- `inputFileCompactions.get(inputFile)` will be always called even the value is not used.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
- Why doesn't it throw an exception instead?
- What is supposed to do when seeing the warn message?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482756457
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
If we put it in DEBUG we won't even know when it has already happened. With this change this log will become very infrequent. This behaviour is supposed to be unexpected where we are adding an entry into the compaction log table without the input sst file being present in the disk. But the consumer of this compaction handles this correctly but we want to know if this happened at all in the system.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482709661
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
Ideally I wouldn't have wanted to build this complex DAG to track all the compactions at the first place and would have wanted to just build a linked list of all the L0 sst flushes by tracking just the sst file flush. This would have made our entire Backup SstFileManagement and diff very straightforward
https://github.com/facebook/rocksdb/blob/36ce2e2a0a4d229d1805766b0e9534b1f708929f/java/src/test/java/org/rocksdb/EventListenerTest.java#L46-L58
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482696887
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -493,14 +493,7 @@ public void onCompactionCompleted(RocksDB db,
builder = new CompactionLogEntry.Builder(trxId,
System.currentTimeMillis(),
inputFileCompactions.keySet().stream()
- .map(inputFile -> {
- if (!inflightCompactions.containsKey(inputFile)) {
- LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
- "compactionBeginListener.",
- inputFile);
- }
- return inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile));
- })
+ .map(inputFile -> inflightCompactions.getOrDefault(inputFile, inputFileCompactions.get(inputFile)))
Review Comment:
Yeah let me convert this to
inputFileCompactions.containsKey(inputFile) ?
inflightCompactions.get(inputFile) : inputFileCompactions.get(inputFile)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2482685522
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
We need not throw exception here since this can only happen when on compactionBegins there were no snapshots and the snapshots exists now. We would have a compaction log entry corresponding to which we won't have a hard linked sst file. This case is ok since we can still do an efficient snapshot diff. However we want to be in a position to debug some other corner case scenario later on so far we have not come across any issue in any of our environments. This can also happen because of some compaction issue in the underlying rocksdb.
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,6 +523,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
+ if (!inflightCompactions.containsKey(inputFile)) {
+ LOG.warn("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.",
+ inputFile);
Review Comment:
We need not throw exception here since this can only happen when on compactionBegins there were no snapshots and the snapshots exists now. We would have a compaction log entry corresponding to which we won't have a hard linked sst file. This case is ok since we can still do an efficient snapshot diff. However we want to be in a position to debug some other corner case scenario later on so far we have not come across any issue in any of our environments. This can also happen because of some compaction issue in the underlying rocksdb. Thus the warn message.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495809728
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,7 +525,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
- inflightCompactions.remove(inputFile);
+ CompactionFileInfo removed = inflightCompactions.remove(inputFile);
+ if (removed == null) {
+ LOG.info("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.", inputFile);
Review Comment:
What is `compactionBeginListener` ? Could you show the code/doc?
Please include the job id and column family name in the message.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495837212
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,7 +525,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
- inflightCompactions.remove(inputFile);
+ CompactionFileInfo removed = inflightCompactions.remove(inputFile);
+ if (removed == null) {
+ LOG.info("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.", inputFile);
Review Comment:
When you say show code/doc do you mean I should add a comment in the code here?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13863. RocksDBCheckpointDiffer Compaction Tracker erroneously logs warn logs in absence of snapshot [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9230:
URL: https://github.com/apache/ozone/pull/9230#discussion_r2495869815
##########
hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java:
##########
@@ -530,7 +525,11 @@ public void onCompactionCompleted(RocksDB db,
compactionLogEntry.getOutputFileInfoList(),
compactionLogEntry.getDbSequenceNumber());
for (String inputFile : inputFileCompactions.keySet()) {
- inflightCompactions.remove(inputFile);
+ CompactionFileInfo removed = inflightCompactions.remove(inputFile);
+ if (removed == null) {
+ LOG.info("Input file not found in inflightCompactionsMap : {} which should have been added on " +
+ "compactionBeginListener.", inputFile);
Review Comment:
I mean -- Could you show the code/doc mentioning `compactionBeginListener`?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org