How to populate request MessageId when deriving from DcmScu
Moderator: Moderator Team
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
Thank you, I will check the patch
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
I'll check it as soon as possible. Right now I am blocked with something else, but it's not forgotten.
BR, Michael
BR, Michael
Re: How to populate request MessageId when deriving from DcmScu
Hi Michael,
Have you had time to look at the PR yet?
Thanks,
Jøger
Have you had time to look at the PR yet?
Thanks,
Jøger
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
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
BR,
Michael
Re: How to populate request MessageId when deriving from DcmScu
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
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
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
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
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
Re: How to populate request MessageId when deriving from DcmScu
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
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
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
Thanks Jøger, it's not forgotten
Re: How to populate request MessageId when deriving from DcmScu
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
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
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
Hi Jøger,
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
True.
Various compilers and operating systems failed, Windows and Linuxes, different compilers.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.
The problem is solved on all our (113, right now) nightly builds.* Do the tests still fail intermittently in their current implementation, or is the problem solved?
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.* 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?
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
Re: How to populate request MessageId when deriving from DcmScu
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
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
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
Hi Jøger,
Best,
Michael
Good ideajogerh 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.
Best,
Michael
Re: How to populate request MessageId when deriving from DcmScu
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
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
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
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
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
-
- DCMTK Developer
- Posts: 2051
- Joined: Fri, 2004-11-05, 13:47
- Location: Oldenburg, Germany
- Contact:
Re: How to populate request MessageId when deriving from DcmScu
Hi Jøger,
Best regards,
Michael
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!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.
Best regards,
Michael
Who is online
Users browsing this forum: Bing [Bot], Google [Bot], Semrush [Bot] and 1 guest