should `/attach` trigger initial logical size calculation, like during timeline create & PS startup warmup?

[problame]

          While reading this, I was wondering: we probably _want_ attach to trigger initial logical size calculation.

Then again, not during bulk migrations. I don't think we ever really thought about that.

So, not a problem with this PR, but, should create a ticket. Will do that and then resolve this conversation.

Originally posted by @problame in https://github.com/neondatabase/neon/pull/6301#discussion_r1447651736

---

Leaving this for discussion during the next bug triage session.

[koivunej]

So right now we assign AttachType::Normal if an attach happens after initial tenant mgr init (regardless if the init is still ongoing). I was thinking this was already implemented, but it isn't.

I wonder if the init order should be just persistent to remove one more option branch.

[koivunej]

I have been trying to wrap my mind around this. As far as implementing this, we would need to complicate enum SpawnMode passed to Tenant::spawn to specify lazy or eager. Lazy or eager would have to be communicated with LocationConf PUTs.

Noted: LocationConf are passed around as plain structs not enums per what kind of rpc is wanted, we also have some invariants which are not maintainained while deserializing.

I wonder if the init order should be just persistent to remove one more option branch.

Those can be worked away by other means.

Lazy vs. eager makes still sense in the mass migration events, however even the lazy must be "eventually activated" because we will not get consumption metrics otherwise. I cannot yet see how should we provide for this. I think the initialization time PageserverConf::concurrent_tenant_warmup semaphore used for background initialization makes sense for this, but it's a rather big step away from it's original meaning -- even if that is just the single if init_order.is_some() guarding it. Well, this doesn't seem so bad, despite of handling the semaphore being closed we never actually do close it.

[jcsp]

we would need to complicate enum SpawnMode passed to Tenant::spawn to specify lazy or eager

That sounds right to me.

Noted: LocationConf are passed around as plain structs not enums per what kind of rpc is wanted, we also have some invariants which are not maintainained while deserializing.

The way I think of it is: LocationConf is the declarative part that says how we want the tenant to end up, and other parameters like flush_ms go as query parameters on the request, and are passed around as separate arguments. So the SpawnMode would be one of those query params rather than part of LocationConf.

even the lazy must be "eventually activated" because we will not get consumption metrics otherwise

Right, this is already covered here: https://github.com/neondatabase/neon/blob/5273c94c59c751cec058a10934a6da94379ba805/pageserver/src/tenant.rs#L717

The semantic is "activate on-demand or opportunistically as units in concurrent_tenant_warmup become available".

[koivunej]

Now good to close as remaining tasks are on the linked cloud issue.