TimeoutLock less HWP PMX agent

[ykyohei]

Description

TimeoutLockless HWP PMX agent

Motivation and Context

In the operation of PMX at the site, we have many timeout lock errors. To avoid this error and make PMX control more robust, we modify the PMX agent in the same way as HWP PCU agent #600
I plan to do the same modification for the HWP PID agent as well.
The way I define actions may not be good. Suggestions for better coding are welcome.

How Has This Been Tested?

Tested in PMX in UTokyo setup

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[jlashner]

Hi Kyohei,

Looks good, but there are a few things off about the task structure here that need to be fixed. Can you fix them, and then I will help to resolve any test errors that still exist? Thanks!

[jlashner]

Hi Kyohei, in the lockless agents, all tasks should be structured something like this:

@defer.inlineCallbacks
  def set_on(self, session, params):
    action = Actions.SetOn(**params)
    self.action_queue.put(action)
    session.data = yield action.deferred
    return True, f"Action {action} finished successfully"

I'd move the shutdown-mode check to either the _process_actions function, or to the action's process function, in which case it should be passed in along with the module.

They also need to be registered with blocking=False, like

agent.register_task('set_on', PMX.set_on, blocking=False)

[jlashner]

If we can test the hardware and failure modes, I'd personally opt for something a bit more robust than this. The LS240 restructure branch has an example of how we can structure this such that on connection failures, it will continue to try making the connection instead of just killing the reactor thread:

socs/socs/agents/lakeshore240/agent.py

Lines 203 to 223 in adf12ff

	module: Optional[Module] = None
	session.set_status('running')
	# Clear pre-existing actions
	while not self.action_queue.empty():
	action = self.action_queue.get()
	action.deferred.errback(Exception("Action cancelled"))
	pm = Pacemaker(self.f_sample, quantize=False)
	while session.status in ['starting', 'running']:
	if module is None:
	try:
	module = self._init_lakeshore()
	except ConnectionRefusedError:
	self.log.error(
	"Could not connect to Lakeshore. "
	"Retrying after 30 sec..."
	)
	time.sleep(30)
	pm.sleep()
	continue

[jlashner]

I'm looking at the test failures here, and it seems like the PMX integration tests rely strongly on the old agent structure, and I don't personally see a clear way to modify them to work with the new structure. I think we should remove them for the time being, and maybe add in tests later that use the HWP Emulator. @BrianJKoopman what do you think?

[ykyohei]

@jlashner I made requested changes, and confirmed that these are working in UTokyo setup. Please review.

[jlashner]

looking good to me! I made one small change (moved the publish / setting of session data into the try-block for the _get_data function), and fixed integration tests so that they can run for me. Hopefully tests pass, and then we can merge!

[jlashner]

Tests pass!! and integration tests were very informative in catching a race condition. I think we can merge now.

[mhasself]

@jlashner This needs to be put in the reactor too ... I suggest blockingCallFromThread.