How to populate request MessageId when deriving from DcmScu

All other questions regarding DCMTK

Moderator: Moderator Team

Message
Author
Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#16 Post by Michael Onken »

Thank you, I will check the patch :)

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#17 Post by Michael Onken »

I'll check it as soon as possible. Right now I am blocked with something else, but it's not forgotten.

BR, Michael

jogerh
Posts: 37
Joined: Mon, 2022-02-28, 08:55

Re: How to populate request MessageId when deriving from DcmScu

#18 Post by jogerh »

Hi Michael,

Have you had time to look at the PR yet?

Thanks,
Jøger

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#19 Post by Michael Onken »

We just tagged the release a few days ago, I will integrate the MPPS N-CREATE patch next week and will also review the N-SET patch then.

BR,
Michael

jogerh
Posts: 37
Joined: Mon, 2022-02-28, 08:55

Re: How to populate request MessageId when deriving from DcmScu

#20 Post by jogerh »

Thanks for integrating the N-CREATE code on master Michael!

I am sorry about the issues with the tests, and I try to understand why the initial design did not work on your build servers. The hypothesis was that a ping client with infinite timeout would never fail to associate with the server, even if the server takes time (even minutes) to start up on a heavily loaded build server. Was it the TCP connect timeout that caused problems or something else?

Thanks,
Jøger

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#21 Post by Michael Onken »

Hi Jøger,

Thank you for the feedback :) Actually I am not so sure what caused the problem, and I tried to fix it by simplifying the tests (for me), and play with timeouts.

Actually, we still have some related errors on our build dashboard on some systems and compilers. It seems to have to do with timing. I plan to look at it this week.
The problem with debugging this is, that if you execute the test directly on the system and compiler that fails, the test completes successfully just fine.

Best regards,
Michael

jogerh
Posts: 37
Joined: Mon, 2022-02-28, 08:55

Re: How to populate request MessageId when deriving from DcmScu

#22 Post by jogerh »

Hi Michael,

https://github.com/DCMTK/dcmtk/pull/57 (Extend DcmScu with support for N-SET requests) is rebased on top of master, and with some slight fixes since the previous draft. In particular, the attributeList of the DcmSCU::sendNSETRequest is made mandatory. This also goes for DcmSCU::sendNCREATEMessage, where I updated the docstring.

Thanks,
Jøger

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#23 Post by Michael Onken »

Thanks Jøger, it's not forgotten :)

jogerh
Posts: 37
Joined: Mon, 2022-02-28, 08:55

Re: How to populate request MessageId when deriving from DcmScu

#24 Post by jogerh »

Hi again Michael,

I am trying to understand why the unit tests failed in your build environment. I believe that if these kind of tests fail on a build server, it is not unlikely that the code could fail also in a real life scenario. Such tests are extremely useful for detecting hard-to-reproduce bugs, and I believe the tests are a great for training purposes. Admittedly, it is a challenge that unit tests involving networking tend to deadlock under failure conditions, which is a problem for continuous integration. Still, I was hoping it would be possible to identify some patterns or principles that would allow us to robustly test SCU/SCP communication also in a CI setting without need for sleep statements or hand tuned timeouts.

* Did you see any pattern across the build servers? Were some operating systems/compilers more likely to fail? I ask this because I have a hunch that for example setting dcmConnectionTimeout to -1 does not mean infinite timeout on Windows platform.
* Do the tests still fail intermittently in their current implementation, or is the problem solved?
* Would it be an idea to introduce a very simple test that by design 'can never fail' in the test suite (unless ports are clashing), and see if they are running robustly?

Thanks,
Jøger

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#25 Post by Michael Onken »

Hi Jøger,
jogerh wrote: Tue, 2022-06-28, 13:06 Hi again Michael,

I am trying to understand why the unit tests failed in your build environment. I believe that if these kind of tests fail on a build server, it is not unlikely that the code could fail also in a real life scenario.
True.
jogerh wrote: Tue, 2022-06-28, 13:06 Such tests are extremely useful for detecting hard-to-reproduce bugs [...]

* Did you see any pattern across the build servers? Were some operating systems/compilers more likely to fail? I ask this because I have a hunch that for example setting dcmConnectionTimeout to -1 does not mean infinite timeout on Windows platform.
Various compilers and operating systems failed, Windows and Linuxes, different compilers.
* Do the tests still fail intermittently in their current implementation, or is the problem solved?
The problem is solved on all our (113, right now) nightly builds.
* Would it be an idea to introduce a very simple test that by design 'can never fail' in the test suite (unless ports are clashing), and see if they are running robustly?
Not sure how such a test should look like since tests are already very simple. BUT: The port clashing is most likely (99% in my opinion) the reason. The tests are executed on a number of virtual machines and as far as I could understand (from the student who takes care of that infrastructure) it could happen that tests from two builds could be executed simultaneously.

When logging into the machines and executing the test manually, the error never showed up. When DCMTK team members executed tests on their own system (Windows and Linux), the errors never showed up.

Actually, these two patches (second only fixes first one) which randomize the port have been solving all problems on the nightly builds from one day to the other:

http://git.dcmtk.org/?p=dcmtk.git;a=com ... caaa5dbaff
http://git.dcmtk.org/?p=dcmtk.git;a=com ... 4c0c52289f

Blaming port clashes also explain the slightly different positions where the code failed, and also that it sometimes went through.

So I'm personally really confident that port clashes on the build server(s) have been causing the problem.

Hope that helps...
Michael

jogerh
Posts: 37
Joined: Mon, 2022-02-28, 08:55

Re: How to populate request MessageId when deriving from DcmScu

#26 Post by jogerh »

Thank you Micahel, this sounds like a good explanation for the problems.

I assume this is a universal challenge for automated testing of DICOM networking functionality. Maybe it could be possible to let the OS assign a free port if the DcmSCP is configured with port 0. DcmSCP seems to already have a function to query the port. I will look into what it would take.

Thanks,
Jøger

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#27 Post by Michael Onken »

Hi Jøger,
jogerh wrote: Tue, 2022-06-28, 14:31 I assume this is a universal challenge for automated testing of DICOM networking functionality. Maybe it could be possible to let the OS assign a free port if the DcmSCP is configured with port 0. DcmSCP seems to already have a function to query the port. I will look into what it would take.
Good idea :)

Best,
Michael

jogerh
Posts: 37
Joined: Mon, 2022-02-28, 08:55

Re: How to populate request MessageId when deriving from DcmScu

#28 Post by jogerh »

Hi Michael,

When you have time, could you please have a look at https://github.com/DCMTK/dcmtk/pull/63? It is an attempt to prevent port conflicts when using DcmSCP in unit tests.

Is it correct that tests marked as EF_Slow are not run on the build servers? Maybe this could explain why the older dcmnet tests did not cause problems before?

Thanks,
Jøger

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#29 Post by Michael Onken »

Hi,

I will look at the pull request. I think it is possible that the EF_Slow mark was the EF_Slow mark caused the tests not having problems. As far as I remember, we now run a few builds with EF_Slow tests but I need to recheck that.

Best regards,
Michael

Michael Onken
DCMTK Developer
Posts: 2048
Joined: Fri, 2004-11-05, 13:47
Location: Oldenburg, Germany
Contact:

Re: How to populate request MessageId when deriving from DcmScu

#30 Post by Michael Onken »

Hi Jøger,
jogerh wrote: Wed, 2022-06-08, 08:58 https://github.com/DCMTK/dcmtk/pull/57 (Extend DcmScu with support for N-SET requests) is rebased on top of master, and with some slight fixes since the previous draft. In particular, the attributeList of the DcmSCU::sendNSETRequest is made mandatory. This also goes for DcmSCU::sendNCREATEMessage, where I updated the docstring.
This is now commited (c9f860) to DCMTK testing branch. Once it goes through the build servers, I will make the commit available in master. I only fixed a typo and initialization using {} in two places (which we prefer not to have in DCMTK). Thanks again for the well-prepared patch!

Best regards,
Michael

Post Reply

Who is online

Users browsing this forum: Semrush [Bot] and 1 guest