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

Context-Scoped Sessions #7198

Open
4 tasks
Stebalien opened this issue Apr 23, 2020 · 3 comments
Open
4 tasks

Context-Scoped Sessions #7198

Stebalien opened this issue Apr 23, 2020 · 3 comments
Labels
kind/enhancement A net-new feature or improvement to an existing feature

Comments

@Stebalien
Copy link
Member

Stebalien commented Apr 23, 2020

Currently, bitswap sessions are hard to work with because we need to:

  1. Create a new session.
  2. Pass this session to all services that need access to the exchange.

Usually 2 involves re-creating these services. This can be very painful and we have many places in go-ipfs where we simply aren't doing this.

Instead, we should associate a session with a context. That way, whenever we use the context, we use the session. When we cancel the context, we cancel the session.

PRs

Design

Store a session "key" in the session to identify the session.

// SessionID is an opaque type uniquely identifying a session.
type SessionID struct {
	// Opaque type to ensure users don't cook this up and break things.
    // Sequentially and atomically allocated.
	id uint64
}

// IsZero returns true if this SessionId is the zero value (not a valid session
// ID).
func (id SessionID) IsZero() bool {
	return id.id == 0
}

// NewSession registers a new session with the context. The session will be
// closed when the passed-in context is canceled.
//
// If there's already a session associated with the context, the existing
// session will be used.
//
// This function does not initialize any state, it just reserves a new SessionID
// associates it with the context.
func NewSession(ctx context.Context) context.Context {
  ...
}

// GetOrCreateSession loads the session from the context, or creates one if
// there is no associated session.
//
// This function also returns the context used to create the session. The
// session should be stopped when this context is canceled.
func GetOrCreateSession(ctx context.Context) (SessionID, context.Context) {
  ...
}

Alternatives

I've considered two alternatives:

  1. Store the exchange/fetcher in the context.
  2. Store an abstract session in the context.

Embedded Exchange

We could:

  1. Create a session.
  2. Store the session/"fetcher" in the context.
  3. Use this session inside the blockservice instead of the actual exchange.

On one hand, this is nice because we can embed alternative exchanges for a specific request. For example, we could embed an offline exchange to perform an offline lookup.

On the other hand:

  1. This is spooky action at a distance.
  2. This only solves this particular problem and doesn't help services exchange information related to sessions.

Abstract Session

We could also store an abstract session object in the context (#6525). However, this would require building a general-purpose, abstract session/state manager. The session-key design lets us do everything the abstract session proposal does, without requiring a general-purpose anything.

@Stebalien Stebalien added the kind/enhancement A net-new feature or improvement to an existing feature label Apr 23, 2020
Stebalien added a commit to ipfs/go-ipfs-exchange-interface that referenced this issue Apr 23, 2020
This patch allows associating a "session key" with a context by calling
`NewSession(ctx)`. When the session is used in a request, the exchange can call
`GetSession(ctx)` to make the request in the appropriate session, or create a
new session with the given session key.

Implements ipfs/kubo#7198
@hannahhoward
Copy link
Contributor

so I gave it a glance over — generally, yes, thumbs up, good plan, seems much closer the web apps uses cookies (which store minimal data, but can be used to lookup other session information). one thing that isn’t totally clear to me: is the code you’ve written out supposed to go in go-bitswap or go-ipfs? If go-bitswap, I wouldn’t want to disable the ability to make sessions manually for some other use case besides go-ipfs (for example, filecoin, which may still use bitswap sessions for blocksync in Lotus? -- not sure there)

Stebalien added a commit to ipfs/go-ipfs-exchange-interface that referenced this issue Apr 23, 2020
This patch allows associating a "session key" with a context by calling
`NewSession(ctx)`. When the session is used in a request, the exchange can call
`GetSession(ctx)` to make the request in the appropriate session, or create a
new session with the given session key.

Implements ipfs/kubo#7198
@Stebalien
Copy link
Member Author

In go-bitswap. I'll leave the NewSession function there as we explore this interface. However, if the context contains a session-key, NewSession will just return the session associated with the key.

Stebalien added a commit to ipfs/go-ipfs-exchange-interface that referenced this issue Apr 23, 2020
This patch allows associating a "session key" with a context by calling
`NewSession(ctx)`. When the session is used in a request, the exchange can call
`GetSession(ctx)` to make the request in the appropriate session, or create a
new session with the given session key.

Implements ipfs/kubo#7198
Stebalien added a commit to ipfs/go-ipfs-exchange-interface that referenced this issue Apr 23, 2020
This patch allows associating a "session key" with a context by calling
`NewSession(ctx)`. When the session is used in a request, the exchange can call
`GetOrCreateSession(ctx)` to make the request in the appropriate session, or create a
new session with the given session key.

Implements ipfs/kubo#7198
@Stebalien
Copy link
Member Author

I had to change the design slightly to use integer IDs:

  1. *struct{} isn't guaranteed to be unique.
  2. I wanted a nice stable string representation. However, *anything can move.

So now I'm just atomically allocating increasing 64bit IDs.

Stebalien added a commit to ipfs/go-bitswap that referenced this issue Apr 23, 2020
Stebalien added a commit to ipfs/go-bitswap that referenced this issue Apr 23, 2020
Stebalien added a commit to ipfs/go-ipfs-exchange-interface that referenced this issue Apr 23, 2020
This patch allows associating a "session key" with a context by calling
`NewSession(ctx)`. When the session is used in a request, the exchange can call
`GetOrCreateSession(ctx)` to make the request in the appropriate session, or create a
new session with the given session key.

Implements ipfs/kubo#7198
Stebalien added a commit to ipfs/go-bitswap that referenced this issue Apr 23, 2020
Stebalien added a commit to ipfs/go-ipfs-exchange-interface that referenced this issue Apr 23, 2020
This patch allows associating a "session key" with a context by calling
`NewSession(ctx)`. When the session is used in a request, the exchange can call
`GetOrCreateSession(ctx)` to make the request in the appropriate session, or create a
new session with the given session key.

Implements ipfs/kubo#7198
Stebalien added a commit that referenced this issue Apr 23, 2020
* Ensure we have a single session per gateway request.
* Use a session in the ipfs refs command.
* Start a session before resolving the pin root (also fixes sessions while
  pinning).
* Update go-merkledag to reliably create sessions.

part of #7198
Stebalien added a commit that referenced this issue Apr 23, 2020
* Ensure we have a single session per gateway request.
* Use a session in the ipfs refs command.
* Start a session before resolving the pin root (also fixes sessions while
  pinning).
* Update go-merkledag to reliably create sessions.

part of #7198
Stebalien added a commit that referenced this issue Apr 23, 2020
* Ensure we have a single session per gateway request.
* Use a session in the ipfs refs command.
* Start a session before resolving the pin root (also fixes sessions while
  pinning).
* Update go-merkledag to reliably create sessions.

part of #7198
Stebalien added a commit that referenced this issue Apr 23, 2020
* Ensure we have a single session per gateway request.
* Use a session in the ipfs refs command.
* Start a session before resolving the pin root (also fixes sessions while
  pinning).
* Update go-merkledag to reliably create sessions.

part of #7198
Stebalien added a commit that referenced this issue Apr 23, 2020
* Ensure we have a single session per gateway request.
* Use a session in the ipfs refs command.
* Start a session before resolving the pin root (also fixes sessions while
  pinning).
* Update go-merkledag to reliably create sessions.

part of #7198
Stebalien added a commit that referenced this issue May 29, 2020
This isn't perfect (we only use sessions after resolving the root cid) but it's
better than what we have. The real solution is #7198 so we can use sessions
everywhere.
Stebalien added a commit that referenced this issue May 29, 2020
This isn't perfect (we only use sessions after resolving the root cid) but it's
better than what we have. The real solution is #7198 so we can use sessions
everywhere.
Stebalien added a commit that referenced this issue Jun 3, 2020
This isn't perfect (we only use sessions after resolving the root cid) but it's
better than what we have. The real solution is #7198 so we can use sessions
everywhere.

(cherry picked from commit 62f61c5)
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 6, 2020
This isn't perfect (we only use sessions after resolving the root cid) but it's
better than what we have. The real solution is ipfs#7198 so we can use sessions
everywhere.
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 8, 2020
This isn't perfect (we only use sessions after resolving the root cid) but it's
better than what we have. The real solution is ipfs#7198 so we can use sessions
everywhere.
ralendor pushed a commit to ralendor/go-ipfs that referenced this issue Jun 8, 2020
This isn't perfect (we only use sessions after resolving the root cid) but it's
better than what we have. The real solution is ipfs#7198 so we can use sessions
everywhere.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants