-
Notifications
You must be signed in to change notification settings - Fork 1
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
HTTP server fixes #15
Closed
Closed
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
869f7ec
track latest changes
FotiosBistas 21c741c
added concurrency control in http server
FotiosBistas 3934195
decrease queue timer http server
FotiosBistas e079972
removed path from get and handler
FotiosBistas acec5ae
counter to debug cid received from get
FotiosBistas 68b0d90
changed position of fetch res
FotiosBistas 8054036
added log messages and removed provider store operations
FotiosBistas aa552a7
fixed log messages for better debugging
FotiosBistas c5454c7
bug fix: for loop not exiting
FotiosBistas 8fba1ed
better testing
FotiosBistas f2a2b1a
avoiding same error
FotiosBistas 4bde352
added missing break
FotiosBistas 2bf2c8a
Bug fix: queue would fill up
FotiosBistas 622c21e
bug fix: shutting down of http server before adding last cids
FotiosBistas 1d3ebe7
Left only post method
FotiosBistas 5dcaa48
shutdown server
FotiosBistas 9a50c6e
simplified code with getNewHttpCid
FotiosBistas 7c5cb87
added comments
FotiosBistas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reasoning behind having a Get method in the API? I thought we just needed to notify with a POST of the CID that we want to track (and the related info)
Am I missing anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @cortze, the functionality of the HTTP server is that a post requests are received by the peer that is publishing, and
encapsulated json
arrays are then added to the queue.This means that we wait for all the providers to be added into the array.
The get method get's called right now of a time interval of 5 seconds and we receive the first element of the queue using a
dequeue
method. I found it easier to be able to determine whether the queue is just empty, inferring that the publisher is just publishing slower than the queue empties itself, or the publisher will not publish any more elements.If the queue is just empty the server responds back with
NoContent
, else if the publisher finished publishing it must send anil
encapsulated json
:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohhhhh I see, I thought that the API was organized in a different way.
My understanding was that we would initialize the API server in the Cid-Hoarder, and that the publisher would be able to POST the CidInfo and the related one to the PRHolders in the same POST query.
But being this POST query after the publication has been accomplished
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried doing it like this but I was impossible to tell whether the publishing process was finished. It also would add a lot of extra logic by checking for each PR which CID it corresponds to and some type of waiting mechanism to add all the PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but are you taking into account having a single json sent over the POST method including the CidInfo and PRHolders?
(something like this)
this would simplify having to match PRHolders with CIDs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is exactly how I'm sending it:
Notice that the
ContentID
,Creator
,ProvideTime
,PublicationTime
stay the same. After that is sent a new batch of provider records for a specificCID
will be sent over the server.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cortze I removed the get method. Essentially the internal queue of the server would fill up and ,at some point later after a lot of CIDs have been inserted, would never empty itself. I replaced it, just like you mentioned at the above comment, with just a post method that sends the received CIDs through a channel and then inserts them into the database. It does manage to insert them all, unlike the previous method which would get stuck, but it still is slow. E.G. the database has inserted 1500/1900 CIDs (numbers are random here) immediately and then the rest 400 CIDs all will get inserted after a couple of hours. So I think some approaches would be:
Would love to hear your thoughts!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @FotiosBistas , thanks for taking into consideration the upper suggestion. Tbh it makes everything cleaner and easier to follow.
About your different approaches: