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

Retry finding the DB Server port twice before giving up. #37

Conversation

@ben-xo
Copy link
Contributor

commented Oct 5, 2019

This gives us a chance to find the port if ConnectionManager jumps in slightly too early after a network disconnection. This seems to help when a CDJ drops of the network then comes back.

I find that this is more responsive if I also lower the socket timeout from 10s to 3s in my application, otherwise it will just take a long time to get back on track.

This gives us a chance to find the port if ConnectionManager jumps in slightly too early after a network disconnection. This seems to help when a CDJ drops of the network then comes back.
@brunchboy

This comment has been minimized.

Copy link
Member

commented Oct 6, 2019

Thanks, I will take a look at this and try to reload these interacting structures into my brain when I get back home to my big monitors and CDJs.

@brunchboy

This comment has been minimized.

Copy link
Member

commented Oct 11, 2019

I haven’t forgotten about this! I am back home, but still have to catch up on many things after my urgent trip, and work is busy and challenging as well. But I plan to spend some time on my open source projects this weekend.

if(!portFound && retryCount > 0) {
requestPlayerDBServerPort(announcement, retryCount - 1);
}
Comment for lines +322  – +324

This comment has been minimized.

Copy link
@brunchboy

brunchboy Oct 11, 2019

Member

Since Java doesn’t have tail-call elimination, I will probably structure this as an ordinary loop, rather than nesting stack frames with recursive calls (not that there are likely to be that many of them I admit).

@@ -283,6 +302,7 @@ private void requestPlayerDBServerPort(DeviceAnnouncement announcement) {
os.write(DB_SERVER_QUERY_PACKET);
byte[] response = readResponseWithExpectedSize(is, 2, "database server port query packet");
if (response.length == 2) {
portFound = true;

This comment has been minimized.

Copy link
@brunchboy

brunchboy Oct 13, 2019

Member

Based on what I found in my own investigation tonight, this would fail with my players, because we do get back a 2 byte response (which would cause portFound to be set to true) but the value we get back is actually -1. So if taking this approach, we would want to test the actual port number in the response. And probably add some sleep time if we get a -1 back.

@brunchboy

This comment has been minimized.

Copy link
Member

commented Oct 13, 2019

Thanks again for all your analysis and ideas! I am going to close this merge request because I went a slightly different direction with it, but the basic approach of trying more than once when we don’t get an answer right away seems to be the solution! I check the actual value returned as well as the length, added some increasing delay between each attempt, and try up to three times. Hopefully that will be more than robust enough. Please let me know how it works in your environment once I have pushed my changes which close the issue.

I also changed what happens when we lose a device. Instead of putting a -1 into its port map entry, I simply remove the device from the map, which makes more sense.

@brunchboy brunchboy closed this Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.