Skip to content

lisafs: move implementation to connections#13158

Closed
shayonj wants to merge 1 commit into
google:masterfrom
shayonj:s/lisafs-per-connection-opts
Closed

lisafs: move implementation to connections#13158
shayonj wants to merge 1 commit into
google:masterfrom
shayonj:s/lisafs-per-connection-opts

Conversation

@shayonj
Copy link
Copy Markdown
Contributor

@shayonj shayonj commented May 12, 2026

The lisafs wire protocol already encodes the supported message list and the max message size per-mount on MountResp, but the server-side implementation reads both off a server-wide ServerImpl and reads ServerOpts off Server, so a custom implementation that needs to advertise different MIDs or run with different "OnDeleted" semantics has to bring its own lisafs.Server. That multi-Server shape carries its own renameMu, filesystem tree, and per-node opsMu instead of sharing them across mounts.

This moves Mount, SupportedMessages, MaxMessageSize, and the former ServerOpts behind a per-Connection ConnectionImpl. Server keeps the shared renameMu, filesystem tree, connection lifecycle, and an opaque implementation value for existing fsgofer config access, while each Connection reads its supported MIDs from its ConnectionImpl and caches hot-path configuration such as max message size and ConnectionOpts at startup. Stock fsgofer behavior is unchanged because LisafsServer remains the server-wide config carrier and hands out a stock connection implementation.

This is a precursor to a follow-up rework of #12950 that drops the multi-Server fan-out and lets LisafsServer choose a per-mount ConnectionImpl that returns the backend ControlFDImpl, all inside the single shared lisafs.Server.

Comment thread pkg/lisafs/connection.go Outdated
@shayonj shayonj force-pushed the s/lisafs-per-connection-opts branch from 9731246 to 24f3a12 Compare May 13, 2026 11:02
@shayonj shayonj changed the title lisafs: move per-mount options to Connection lisafs: move implementation to connections May 13, 2026
Comment thread pkg/lisafs/connection.go Outdated
Comment thread pkg/lisafs/connection_test.go Outdated
@shayonj shayonj force-pushed the s/lisafs-per-connection-opts branch from 24f3a12 to e6e41a0 Compare May 13, 2026 22:55
Comment thread runsc/fsgofer/lisafs.go Outdated
@shayonj shayonj force-pushed the s/lisafs-per-connection-opts branch from e6e41a0 to d868279 Compare May 13, 2026 23:16
Copy link
Copy Markdown
Collaborator

@ayushr2 ayushr2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Could you resolve merge conflict so I can pull it and fix it internally?

@shayonj shayonj force-pushed the s/lisafs-per-connection-opts branch 2 times, most recently from 9547dcc to 7744544 Compare May 14, 2026 00:09
Copy link
Copy Markdown
Collaborator

@EtiennePerot EtiennePerot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use any over interface{} throughout this PR

Move mount behavior and advertised LisaFS protocol configuration from the
server implementation to a per-connection implementation. Server keeps only
shared coordination state plus an opaque implementation value for handlers
that need server-wide state, while each Connection reads its supported
messages from ConnectionImpl and caches hot-path configuration such as max
message size and options at startup.

Rename ServerOpts to ConnectionOpts and expose it through ConnectionImpl so
backend-specific semantics are selected together with the connection
implementation. Remove unused fsgofer Config fields that were not
server-wide.
@shayonj shayonj force-pushed the s/lisafs-per-connection-opts branch from 7744544 to ead4588 Compare May 14, 2026 01:17
@shayonj
Copy link
Copy Markdown
Contributor Author

shayonj commented May 14, 2026

ah! old habits :). Updated!

copybara-service Bot pushed a commit that referenced this pull request May 14, 2026
The lisafs wire protocol already encodes the supported message list and the max message size per-mount on MountResp, but the server-side implementation reads both off a server-wide ServerImpl and reads ServerOpts off Server, so a custom implementation that needs to advertise different MIDs or run with different "OnDeleted" semantics has to bring its own lisafs.Server. That multi-Server shape carries its own renameMu, filesystem tree, and per-node opsMu instead of sharing them across mounts.

This moves Mount, SupportedMessages, MaxMessageSize, and the former ServerOpts behind a per-Connection ConnectionImpl. Server keeps the shared renameMu, filesystem tree, connection lifecycle, and an opaque implementation value for existing fsgofer config access, while each Connection reads its supported MIDs from its ConnectionImpl and caches hot-path configuration such as max message size and ConnectionOpts at startup. Stock fsgofer behavior is unchanged because LisafsServer remains the server-wide config carrier and hands out a stock connection implementation.

This is a precursor to a follow-up rework of #12950 that drops the multi-Server fan-out and lets LisafsServer choose a per-mount ConnectionImpl that returns the backend ControlFDImpl, all inside the single shared lisafs.Server.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#13158 from shayonj:s/lisafs-per-connection-opts ead4588
PiperOrigin-RevId: 915155303
copybara-service Bot pushed a commit that referenced this pull request May 14, 2026
The lisafs wire protocol already encodes the supported message list and the max message size per-mount on MountResp, but the server-side implementation reads both off a server-wide ServerImpl and reads ServerOpts off Server, so a custom implementation that needs to advertise different MIDs or run with different "OnDeleted" semantics has to bring its own lisafs.Server. That multi-Server shape carries its own renameMu, filesystem tree, and per-node opsMu instead of sharing them across mounts.

This moves Mount, SupportedMessages, MaxMessageSize, and the former ServerOpts behind a per-Connection ConnectionImpl. Server keeps the shared renameMu, filesystem tree, connection lifecycle, and an opaque implementation value for existing fsgofer config access, while each Connection reads its supported MIDs from its ConnectionImpl and caches hot-path configuration such as max message size and ConnectionOpts at startup. Stock fsgofer behavior is unchanged because LisafsServer remains the server-wide config carrier and hands out a stock connection implementation.

This is a precursor to a follow-up rework of #12950 that drops the multi-Server fan-out and lets LisafsServer choose a per-mount ConnectionImpl that returns the backend ControlFDImpl, all inside the single shared lisafs.Server.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#13158 from shayonj:s/lisafs-per-connection-opts ead4588
PiperOrigin-RevId: 915155303
copybara-service Bot pushed a commit that referenced this pull request May 14, 2026
The lisafs wire protocol already encodes the supported message list and the max message size per-mount on MountResp, but the server-side implementation reads both off a server-wide ServerImpl and reads ServerOpts off Server, so a custom implementation that needs to advertise different MIDs or run with different "OnDeleted" semantics has to bring its own lisafs.Server. That multi-Server shape carries its own renameMu, filesystem tree, and per-node opsMu instead of sharing them across mounts.

This moves Mount, SupportedMessages, MaxMessageSize, and the former ServerOpts behind a per-Connection ConnectionImpl. Server keeps the shared renameMu, filesystem tree, connection lifecycle, and an opaque implementation value for existing fsgofer config access, while each Connection reads its supported MIDs from its ConnectionImpl and caches hot-path configuration such as max message size and ConnectionOpts at startup. Stock fsgofer behavior is unchanged because LisafsServer remains the server-wide config carrier and hands out a stock connection implementation.

This is a precursor to a follow-up rework of #12950 that drops the multi-Server fan-out and lets LisafsServer choose a per-mount ConnectionImpl that returns the backend ControlFDImpl, all inside the single shared lisafs.Server.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#13158 from shayonj:s/lisafs-per-connection-opts ead4588
PiperOrigin-RevId: 915155303
@ayushr2
Copy link
Copy Markdown
Collaborator

ayushr2 commented May 14, 2026

@shayonj I did some more surgery, please take a look at #13180

I essentially got rid of the entire ServerImpl thing. Having both ConnectionImpl and ServerImpl was redundant and complex. All the ServerImpl was doing was holding server-wide configuration. The ConnectionImpl can do that too:

type connectionImpl struct {
    serverSettings *Settings
    // other connection-specific settings
}

@shayonj
Copy link
Copy Markdown
Contributor Author

shayonj commented May 14, 2026

Thank you for the thorough review as usual. I will take it there and close this. Let me know if you'd like to keep this open and happy to as well.

@shayonj shayonj closed this May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants