Comment 1 Linn 2022-03-14 12:43:52 CET

Another line in sessionstore.py that could potentially cause similar issues is this:
> # pre-4.8.0 to 4.8.0 upgrade
> sessinfo['status'] = 'normal'

This was added in commit r32121 which handled marking sessions as unreachable.

Comment 2 William Sjöblom 2022-03-28 13:02:33 CEST

The fact that both the 'status' and 'connectionstatus' are always set regardless of their previous value seems to come from the fact that previously, these values were not used unless verification has already occurred. The values set here will thus not be used since session verification will overwrite them both.

Now when we have `tlctl session list', we can potentially use these values before verification has occurred. Thus, if these two keys are already present in the `sessinfo' dict, we should not overwrite these. Since all other operations using these values updates these before they are used (through verification), the only component that should be impacted by such a change is `tlctl'.

Comment 4 William Sjöblom 2022-03-28 14:15:15 CEST

I have tested build #2539 on Fedora 35 and the status fields now survive vsmserver restarts!

Marking as resolved.

Comment 5 Pierre Ossman 2022-03-30 14:54:46 CEST

Seem to work well with a new server. Tested on RHEL 7.

* Created three sessions, two connected and one disconnected. Status before and after restart of vsmserver showed the proper values (2 connected, 1 disconnected).

* Stopped vsmagent and forced a refresh of one session so that it became unreachable. Shown status is unreachable both before and after restarting vsmserver.

However I'm not seeing the expected behaviour on an upgrade.

I'm upgrading from 4.7.0, so "connectionstatus" is already there but "status" needs to be filled in.

1. Start three sessions under 4.7.0
2. Check in tlwebadm that all three have a connection status
3. Upgrade to 4.14.0post
4. tlctl session list

At this point the status is just blank, which is not what I'm expecting. They update to connected eventually though.

Comment 6 William Sjöblom 2022-03-31 12:11:53 CEST

I was able to reproduce the issue mentioned in comment 5 on RHEL7 with only one connected session during the upgrade.

Comment 7 William Sjöblom 2022-03-31 14:59:15 CEST

This seems to be a combination of bug 7877 along with the fact that the "connectionstatus" field is not set at session startup in 4.7.0 and below (fixed in r32075 as part of bug 5489 and shipped in 4.8.0).

Still, we should probably look into using something other than the empty string for "connectionstatus" when upgrading. We already use "unknown" for this field when a session become unreachable which sounds like a reasonable default when upgrading the session store. 

The client is the only thing in the product with control/data flow dependent on the "connectionstatus" field. Since the client also verifies sessions before acting on this data (which overwrites it), changing the "connectionstatus" upgrade default from "" to "unknown" seems very much safe.

Comment 9 William Sjöblom 2022-03-31 16:44:10 CEST

"connectionstatus" will now be assigned "unknown" instead of "" during session store upgrades.

When upgrading from 4.7.0 to build #2547 (as described in comment 5) the native client session list and tlwebadm still works as intended, while `tlctl session list' now shows "unknown" instead of the empty string in the STATUS column.

Marking as resolved.

Comment 10 Pierre Ossman 2022-04-01 15:51:50 CEST

Retested on RHEL 7:

 * legacy sessions now get "unknown" until their first check

 * connected/disconnected is preserved over vsmserver restart

 * unreachable is preserved over vsmserver restart

I also managed to check that the upgrade works, despite bug 7877. I forced a dump to disk by creating a new session. At that point connections that had a proper "connectionstate" saved were correctly shown in tlctl directly after upgrade, and the sessions lacking a state were shown as "unknown".