Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequence Diagram misplaces Handlers #570

Closed
cjoergensen opened this issue Mar 16, 2016 · 24 comments · Fixed by #640
Closed

Sequence Diagram misplaces Handlers #570

cjoergensen opened this issue Mar 16, 2016 · 24 comments · Fixed by #640
Assignees
Milestone

Comments

@cjoergensen
Copy link

cjoergensen commented Mar 16, 2016

Symptoms

The Sequence Diagram misplaces the handler boxes in occasions and that results the message to appear out of order.

screen1

Who's affected

This update is recommended for all users of ServiceInsight who rely on Sequence Diagram as it draws a more accurate sequence of messages in certain situations.

@HEskandari
Copy link
Contributor

@cjoergensen Would you be able to test this on the latest release of 1.5.6? I think this was fixed in 2a06575 and is a duplicate of #560.

@cjoergensen
Copy link
Author

Here is the sequence diagram from 1.5.6:

screeen

@HEskandari
Copy link
Contributor

@cjoergensen Would you be able to send us the ServiceControl database? If you want I can share a folder on Google drive for you to upload it.

@cjoergensen
Copy link
Author

sure thing. My email is: cgj(at)seas-nve.dk

@HEskandari
Copy link
Contributor

@cjoergensen Email set. Please let me know when you have uploaded it. Thanks.

@HEskandari
Copy link
Contributor

HEskandari commented Jul 1, 2016

@cjoergensen I tried to restore it but it seems to be just too big for Raven Studio to handle. Would you be able to upload the DB folder content as per this page into the Google drive please? That way with a simple file copy, I can restore your state and wouldn't have to go through the import process. Thanks.

Just an update that I tried Raven Smuggler as well...after a few hours it also blew up and failed to restore the database.

@HEskandari HEskandari removed this from the 1.5.7 milestone Jul 11, 2016
@HEskandari
Copy link
Contributor

ping @cjoergensen

@cjoergensen
Copy link
Author

Im on vacation, but i will send you the data when im back.

@cjoergensen
Copy link
Author

@HEskandari I have uploaded the audit data. Have a look at messageid = 'a3f893de-25e0-4838-8d13-a655009bc7df'

@HEskandari
Copy link
Contributor

I'll have a look over the weekend as I'll be at the NDC conf for the rest of the week.

@HEskandari
Copy link
Contributor

@cjoergensen I finally got it up and running. I can see the diagram with that message id. I can see that the Started message was at 2016-08-01T07:27:05.912881Z and the Completed message was processed at 2016-08-01T07:27:10.936081Z.

image

With the fix you'll still see the two boxes for the Handlers but the order would be fixed.

@HEskandari
Copy link
Contributor

@cjoergensen this is the diagram generated with the fix. Does this look right now?

image

@cjoergensen
Copy link
Author

@HEskandari Looks great!

@HEskandari HEskandari changed the title Sequence Diagram Misplaces "Processing" boxes Sequence Diagram Misplaces "Handler" boxes Aug 18, 2016
@HEskandari HEskandari changed the title Sequence Diagram Misplaces "Handler" boxes Sequence Diagram misplaces Handlers Aug 18, 2016
@HEskandari HEskandari self-assigned this Aug 18, 2016
@HEskandari HEskandari added this to the 1.5.8 milestone Aug 18, 2016
@HEskandari
Copy link
Contributor

@cjoergensen I have pushed this out to our staging, do you think you can have a look at this pre-release and see if it works as expected?

@cjoergensen
Copy link
Author

cjoergensen commented Aug 19, 2016

@HEskandari Sure thing, but the link is broken...

@HEskandari
Copy link
Contributor

@cjoergensen The link works for me, might be a permission thing, see if this direct download link works?

@HEskandari
Copy link
Contributor

@cjoergensen any updates? I'm waiting for your final confirmation to release the version, thanks beforehand :)

@cjoergensen
Copy link
Author

@HEskandari i keep getting 404 Error page when i click the links.

@HEskandari
Copy link
Contributor

@cjoergensen Apologies. Must be a permission thing. I'll upload it to the google drive and send you the link.

@HEskandari HEskandari reopened this Sep 6, 2016
@HEskandari
Copy link
Contributor

@cjoergensen I have reviewed the updated database. I can see a couple of weird sorting issues in it. I have reopened the issue.

@HEskandari
Copy link
Contributor

@cjoergensen I'm back on this issue. The more I look at it, the more it feels like a time being out of sync between the endpoints. Did you at any point checked if the time between your endpoints are synced, preferably through an NTP service to make sure they will remain in sync?

@HEskandari
Copy link
Contributor

HEskandari commented Oct 20, 2016

Just to summarize where we're at with this issue - as it has been open for some time - here's the current status:

  • The initial discrepancy reported was a bug in ordering of messages in the sequence diagram. That bug was reproduced with the database @cjoergensen provided and was patched. Changes are already merged with the master branch and will be included in the next release.
  • When testing with a pre-release on a different database (updated?), we saw another issue.
  • We're trying to see if the second issue is potentially a time sync issue, since on the other customer databases I have tested, I haven't seen that behavior.

@adamralph adamralph modified the milestones: Future, 1.5.8 Nov 3, 2016
@adamralph
Copy link
Contributor

Since we are reopening closed bugs on the 1.5.8 milestone due to reverting master to 1.5.7 in #617, this issue has been moved from the 1.5.8 milestone to the Future milestone, to make a distinction.

This may be re-added to the 1.5.8 milestone if we decide to fix it before releasing 1.5.8.

@HEskandari
Copy link
Contributor

Hi @cjoergensen were you able to have another look at this? Should we proceed with releasing the interim bug fix, since the existing PR addresses the original issue you raised?

@distantcam distantcam added this to the 1.6.2 milestone Feb 28, 2017
@adamralph adamralph removed this from the 1.6.2 milestone Mar 6, 2017
@adamralph adamralph added this to the 1.6.3 milestone Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants