Bug 7860 - Connection status for sessions gets empty value after vsmserver restart
Summary: Connection status for sessions gets empty value after vsmserver restart
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: VSM Server (show other bugs)
Target Milestone: 4.15.0
Assignee: William Sjöblom
Keywords: ossman_tester, prosaic
Depends on:
Blocks: 425
  Show dependency treegraph
Reported: 2022-03-14 12:42 CET by Linn
Modified: 2022-04-01 15:51 CEST (History)
1 user (show)

See Also:
Acceptance Criteria:


Description Linn cendio 2022-03-14 12:42:04 CET
This was discovered through listing sessions with tlctl (bug 425), since we here show the session info without asking vsmserver to verify the sessions. 

After restarting the vsmserver service, we see that connection status (STATUS) now is empty:
> 09:19:39 [linma@linma tlctl]$ sudo /opt/thinlinc/sbin/tlctl session list
> USER    STATUS     AGENT      PID      DISPLAY  
> ==============================================
> tester  connected  2072392  10       
> Listed 1 session(s).
> 09:19:41 [linma@linma tlctl]$ sudo systemctl restart vsmserver.service 
> 09:20:03 [linma@linma tlctl]$ sudo /opt/thinlinc/sbin/tlctl session list
> ===========================================
> tester  2072392  10       
> Listed 1 session(s).
It seems like the issue is connected to this line in sessionstore.py, which has been around since connection status first was introduced in 2014 (r28397). 
> # pre-4.2,0 to 4.2.0 compliance
> sessinfo['connectionstatus'] = ""
I looked closer if this issue was related to the Python 3 upgrade of sessionstore, but I didn't find anything in the commits to indicate this.
Comment 1 Linn cendio 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 cendio 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 cendio 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 cendio 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 cendio 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 cendio 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 cendio 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 cendio 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".

Note You need to log in before you can comment on or make changes to this bug.