ForestDB - After a failed compaction, the original file cannot be re-compacted

This is a regression and the details are in the e-mail chain

After regression is fixed, as you expected, "1. Forestdb should work fine with fdb.data.1” should happen, allowing it to compact to fdb.data.2

Thanks and with warm regards,
Sundar

From: Deepkaran Salooja <Deepkaran@couchbase.com>
Date: Tuesday, October 17, 2017 at 2:35 PM
To: Sundar Sridharan <sundar@couchbase.com>
Cc: Srinath Duvuru <srinath.duvuru@couchbase.com>, Tai Tran <Tai.Tran@couchbase.com>
Subject: Re: MB-22718

Thanks for the details Sundar. Looks like this is a must fix then.

Once the regression gets fixed, what would be the behavior for 1 and 2 in case of process crash?

Thanks,
Deep

On Oct 17, 2017, at 2:25 PM, Sundar Sridharan <Sundar@couchbase.com> wrote:

From looking at the regression, it looks like even if compaction fails, it is not possible to retry compaction on the old file with the same new filename.
This means that if fdb.data.1 -> fdb.data.2 fails. Retrying compaction of fdb.data.1 -> fdb.data.2 will return INVALID_ARGS due to the regression that Srinath found. The only way to proceed would be to try fdb.data.1 -> fdb.data.3 !
Indexer may not even need to crash, a disk full or any other compaction failure would result in same issue if we are hitting this regression where the new file linkage is being fixed up regardless of the compaction recovery status.

Thanks and warm regards,
Sundar

From: Deepkaran Salooja <Deepkaran@couchbase.com>
Date: Tuesday, October 17, 2017 at 2:19 PM
To: Sundar Sridharan <sundar@couchbase.com>
Cc: Srinath Duvuru <srinath.duvuru@couchbase.com>, Tai Tran <Tai.Tran@couchbase.com>
Subject: Re: MB-22718

Looks like CBSE-4267 is evidence that 2. Next compaction failure with "invalid args” is what happens.

In CBSE-4267, there is no crash/restart of the indexer process.
We want to understand the behavior when the process crashes after successful compaction but before unlinking the file.

Thanks,
Deep

On Oct 17, 2017, at 2:02 PM, Sundar Sridharan <Sundar@couchbase.com> wrote:

Hi Deep,

Looks like CBSE-4267 is evidence that 2. Next compaction failure with "invalid args” is what happens.

Hi Srinath,

Thanks for identifying the change that caused the linkage to be broken. I think regardless of the workaround, we should add a fix to only update the file linkage if the _fdb_recover_compaction() method returns success.

Hi Tai,

This appears to be a regression introduced in 4.6. I think we should include it in the vulcan or the next release of forestdb.

Thanks and with warm regards,
Sundar

From: Deepkaran Salooja <Deepkaran@couchbase.com>
Date: Tuesday, October 17, 2017 at 12:46 PM
To: Srinath Duvuru <srinath.duvuru@couchbase.com>
Cc: Sundar Sridharan <sundar@couchbase.com>, Tai Tran <Tai.Tran@couchbase.com>
Subject: Re: MB-22718

Hi Srinath,

One thing to note is that forestdb will never encounter the situation of 2 files on startup
as Indexer makes sure it cleans up the higher version file before fdb_open if there are 2 files.

One issue I see is, when two files are present and if we use the old file and compact it again, that compaction fails.

What happens if there were 2 files lets say fdb.1 and fdb.2 (compaction succeeded but lets say the process crashed before unlinking the file).
Now Indexer will delete fdb.2 on startup and open fdb.1:
1. Will forestdb be able to work fine with fdb.1?
2. Will the next compaction with new file name as fdb.2 succeed or result in “invalid args”?

Thanks,
Deep

On Oct 17, 2017, at 9:50 AM, Srinath Duvuru <srinath.duvuru@couchbase.com> wrote:

I am not sure if I understand the problem enough to comment.
What forestdb does during compaction is to set the new file name in the old file, do the compaction creating the new file, unlink the old file if there are outstanding references to it (so the indexer that is holding onto old file, can continue to use it but its resources will get cleaned up when there are no references) and then set the old file name in the new file.
So, if we encounter 2 files, there was an error with compaction and the new file is not to be relied upon. We must delete the new file and use the old file in that case.

One issue I see is, when two files are present and if we use the old file and compact it again, that compaction fails. This was due to a change

https://github.com/couchbase/forestdb/blob/spock/src/forestdb.cc#L2194

That change came in with
https://github.com/couchbase/forestdb/commit/e4615599aefa910df049bf29f092987a92d96e55#diff-f6420a1ae26da2202404469c632c0b2c

Please let me know what I am missing.

Thanks.
Srinath.