Re: Failing assertion while taking a restartpoint during crash recovery

Hi

I was looking into the patch, and I was wondering if we could make use
of the following small changes.

* Instead of setting the shared state in StartupXLOG, why not set it
in the StartupSUBTRANS itself?
```
@@ -6234,7 +6234,6 @@ StartupXLOG(void)
* during recovery and need not be started yet.
*/
StartupSUBTRANS(oldestActiveXID);
- SetRecoverySubtransInitialized();
```

* This way we can also add an assertion that subtrans shouldn't
already be initialized.
```
@@ -307,6 +307,8 @@ StartupSUBTRANS(TransactionId oldestActiveXID)
LWLock *prevlock = NULL;
LWLock *lock;

+ Assert(!RecoverySubtransInitialized());
+
/*
* Since we don't expect pg_subtrans to be valid across crashes, we
* initialize the currently-active page(s) to zeroes during startup.
@@ -339,6 +341,8 @@ StartupSUBTRANS(TransactionId oldestActiveXID)
}

LWLockRelease(lock);
+
+ SetRecoverySubtransInitialized();
}
```

* Also we can add an assertion while truncating subtrans for safety.

```
@@ -405,6 +409,8 @@ TruncateSUBTRANS(TransactionId oldestXact)
{
int64 cutoffPage;

+ Assert(RecoverySubtransInitialized());
+
```

* There is is another subtrans call under StartupXLOG that can also be improved
```
@@ -6518,7 +6517,7 @@ StartupXLOG(void)
* Start up subtrans, if not already done for hot standby. (commit
* timestamps are started below, if necessary.)
*/
- if (standbyState == STANDBY_DISABLED)
+ if (standbyState == STANDBY_DISABLED && !RecoverySubtransInitialized())
StartupSUBTRANS(oldestActiveXID);
```

* Other than that we also call TruncateSUBTRANS() while creating a
checkpoint; maybe we can also improve the guard here, although the
assertion under TruncateSUBTRANS could be enough?

@@ -7879,7 +7878,7 @@ CreateCheckPoint(int flags)
* in subtrans.c). During recovery, though, we mustn't do this because
* StartupSUBTRANS hasn't been called yet.
*/
- if (!RecoveryInProgress())
+ if (!RecoveryInProgress() && RecoverySubtransInitialized())
TruncateSUBTRANS(GetOldestTransactionIdConsideredRunning());

Thoughts?

Thanks,
Imran Zaheer