This repository has been archived by the owner on Aug 19, 2022. It is now read-only.
implement the RemovePeer method #174
Merged
Conversation
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
marten-seemann
commented
Oct 23, 2021
pstoreds/metadata.go
Outdated
| @@ -71,3 +71,7 @@ func (pm *dsPeerMetadata) Put(p peer.ID, key string, val interface{}) error { | |||
| } | |||
| return pm.ds.Put(k, buf.Bytes()) | |||
| } | |||
|
|
|||
| func (pm *dsPeerMetadata) RemovePeer(p peer.ID) { | |||
| pm.ds.Delete(pmBase.ChildString(base32.RawStdEncoding.EncodeToString([]byte(p)))) | |||
There was a problem hiding this comment.
This doesn't work. When Putting, we generate a ds.Key based on the peer ID and the key. Without the key, we have no way of getting back (and deleting) that entry. @vyzo, any idea what to do here?
There was a problem hiding this comment.
I think we might need to change the way we generate the ds.Key so that we only use the peer ID.
Why were we using the key in the first place?
There was a problem hiding this comment.
I assume it was implemented so we can store multiple key-value pairs for the same peer. How else would we implement this?
vyzo
reviewed
Oct 23, 2021
pstoreds/metadata.go
Outdated
| @@ -71,3 +71,7 @@ func (pm *dsPeerMetadata) Put(p peer.ID, key string, val interface{}) error { | |||
| } | |||
| return pm.ds.Put(k, buf.Bytes()) | |||
| } | |||
|
|
|||
| func (pm *dsPeerMetadata) RemovePeer(p peer.ID) { | |||
| pm.ds.Delete(pmBase.ChildString(base32.RawStdEncoding.EncodeToString([]byte(p)))) | |||
There was a problem hiding this comment.
I think we might need to change the way we generate the ds.Key so that we only use the peer ID.
Why were we using the key in the first place?
|
but how would we have multiple keys for the same peer ID?
…On Sat, Oct 23, 2021, 17:51 Marten Seemann ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pstoreds/metadata.go
<#174 (comment)>
:
> @@ -71,3 +71,7 @@ func (pm *dsPeerMetadata) Put(p peer.ID, key string, val interface{}) error {
}
return pm.ds.Put(k, buf.Bytes())
}
+
+func (pm *dsPeerMetadata) RemovePeer(p peer.ID) {
+ pm.ds.Delete(pmBase.ChildString(base32.RawStdEncoding.EncodeToString([]byte(p))))
I assume it was implemented so we can store multiple key-value pairs for
the same peer. How else would we implement this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#174 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAI4SQXVY5HCXZDXACN6QDUILDWBANCNFSM5GSHYQOQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
oh these are metadata keys, my bad. How about we use a composite key od the
form `/peerID/key` and do a range scan?
…On Sat, Oct 23, 2021, 18:11 Dimitris Vyzovitis ***@***.***> wrote:
but how would we have multiple keys for the same peer ID?
On Sat, Oct 23, 2021, 17:51 Marten Seemann ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In pstoreds/metadata.go
> <#174 (comment)>
> :
>
> > @@ -71,3 +71,7 @@ func (pm *dsPeerMetadata) Put(p peer.ID, key string, val interface{}) error {
> }
> return pm.ds.Put(k, buf.Bytes())
> }
> +
> +func (pm *dsPeerMetadata) RemovePeer(p peer.ID) {
> + pm.ds.Delete(pmBase.ChildString(base32.RawStdEncoding.EncodeToString([]byte(p))))
>
> I assume it was implemented so we can store multiple key-value pairs for
> the same peer. How else would we implement this?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#174 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAI4SQXVY5HCXZDXACN6QDUILDWBANCNFSM5GSHYQOQ>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
|
marten-seemann
force-pushed
the
remove-peer
branch
2 times, most recently
from
6ba12a1
to
26a66ed
Compare
|
Fixed. We're now querying the keystore for the key prefix and remove all entries. |
vyzo
approved these changes
Oct 24, 2021
go.mod
Outdated
| @@ -12,7 +12,7 @@ require ( | |||
| github.com/ipfs/go-ds-leveldb v0.4.2 | |||
| github.com/ipfs/go-log v1.0.5 | |||
| github.com/libp2p/go-buffer-pool v0.0.2 | |||
| github.com/libp2p/go-libp2p-core v0.8.6 | |||
| github.com/libp2p/go-libp2p-core v0.11.1-0.20211023074228-0b9196b76a61 | |||
There was a problem hiding this comment.
we want a master commit or release tag for this.
marten-seemann
force-pushed
the
remove-peer
branch
from
26a66ed
to
973c2a6
Compare
marten-seemann
force-pushed
the
remove-peer
branch
from
973c2a6
to
2767c2e
Compare
marten-seemann
force-pushed
the
remove-peer
branch
from
2767c2e
to
64c9730
Compare
marten-seemann
force-pushed
the
remove-peer
branch
from
64c9730
to
a3a997d
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Depends on #173.
This PR doesn't fully work yet, removing from the
dsPeerMetaDatadoesn't work (see comment below).