-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
Q: Support for multi-tenant ? #123
Comments
as stated in the docs, multi-tenancy with db per tenant is currently not supported. and would require a significant investment to make it work and would have to break existing API. migrations are gonna be a troublesom also. and i've seen people mentioning there's a noticeable performance degradation when the number of databases in a single server is high. some sort of contention happening in a global level if i remember correctly. however, i will investigate it when i get some time. if you have any ideas, feel free to give it a go and submit a PR. |
Hi. Of course the idea is not be to have "50" databases. As a real case, we host a wepapi and want to have data isolation between the two or three branches Looking at doc on Finbuckle, in the MongoFramework module, they have data isolation with one tenant per database and/or use a globalfilter to isolate tenant inside a database; in other words, the tenant info is prepend when fetching data. They also support hybrid, since each tenant have a connection string. I'll fork the repo a make an attempt. My initial thought would be to leverage the DatabaseFor idea. If an entity is decorated with a MultiTenant attribute, to include an extra "optional key" for indexing the connection string + a global filtering.... To be continued! |
we also have global filters: https://mongodb-entities.com/wiki/DB-Instances-Global-Filters.html |
I agree. I think this can be achieved using a DbContext and resolve the correct db on a "per request" basis. Extracting the tenant info and creating a proper dbcontext, if using a repository. More thoughts in progress. |
so... did a massive refactor to support db per tenant multi-tenancy support. basically the usage will go like this: public class Author : Entity
{
public string Name { get; set; }
}
public class Book : Entity
{
public string Title { get; set; }
} //set which entity types should be stored in which dbs at app startup
//only needed when storing different entities in different databases/servers
//if all types are stored in a single database, this can be skipped
DB.DatabaseFor<Author>("AuthorData");
DB.DatabaseFor<Book>("BookData");
//configure a context for tenant A
//this should be done per request.
//tenant name/prefix will come from the current request.
var ctxForTenantA = new TenantContext("TenantA");
ctxForTenantA.Init("AuthorData", "localhost"); //init calls can alternatively be done at startup if you have a list of tenant names/prefixes
ctxForTenantA.Init("BookData", "localhost");
//configure a context for tenant B
var ctxForTenantB = new TenantContext("TenantB");
ctxForTenantB.Init("AuthorData", "localhost");
ctxForTenantB.Init("BookData", "localhost");
//save entities for tenant A
var tenantABook1 = new Book { Title = "tenant a book 1" };
await ctxForTenantA.SaveAsync(tenantABook1);
var tenantAAuthor1 = new Author { Name = "tenant a author 1" };
await ctxForTenantA.SaveAsync(tenantAAuthor1);
//save entities for tenant B
var tenantBBook1 = new Book { Title = "tenant b book 1" };
await ctxForTenantB.SaveAsync(tenantBBook1);
var tenantBAuthor1 = new Author { Name = "tenant b author 1" };
await ctxForTenantB.SaveAsync(tenantBAuthor1); |
why not reuse the same approach from ef core? so you can put tenancy info in the context, also that means that type to database mapping (DB. DatabaseFor) should be useless too. as for relationships, ef core uses proxies and dynamic type generation to solve that |
Hi @dj-nitehawk, going to be trying that shortly. As for the relationships, maybe we can have a method on the .AddChildrenAsync( parent, child/children ); //for one-many .AddRelatedAsync( parent.child, child/children ); //for many-many |
@ahmednfwela well, true... but the static stuff is here to stay. cause i have around 10-12 projects using those and in no way i'm gonna touch those codebases anytime soon. so breaking the api is out of the question. also, you can sorta achieve a similar thing with a derived var builder = WebApplication.CreateBuilder();
builder.Services.AddScoped<MyDBContext>();
public class MyDBContext : TenantContext
{
public MyDBContext(IConfiguration config, HttpContext httpCtx)
{
SetTenantPrefix(httpCtx.Request.Headers["tenant-id"]);
Init("AuthorData", config["Databse:Authors:HostName"]);
Init("BookData", config["Databse:Books:HostName"]);
}
} @bobbyangers i'll have another look at relationships soon. brain turned into mush trying to achieve multi-tenancy without breaking existing api ;-) |
@dj-nitehawk there is no need to break existing api, it can simply wrap around a static singleton instance of the DBContext |
@ahmednfwela that is exactly what the TenantContext does. what issue do you see with the scoped tenant context approach i posted above? it's simple enough right? |
you can still see static fields and classes
this type to DB map should be done per DBContext, not statically. |
yes took a different approach in the multi-tenancy branch |
yes, but the fact that we are storing anything at all in the static DB class |
The current DBContext api relies on the static DB class, but it should be the other way around |
maybe around v25 we could make it DBContext only and get rid of the static methods. but the static methods are just sooo convenient to use. let's see... if i get a full week of time off from my regular job, i'll look into a full revamp. until then, no breaking changes ;-) |
Great ! I just wanted to see if you agree to the changes in general. |
awesome! hopefully there won't be too many breaking changes to existing consumers... |
As @ahmednfwela was suggesting.... If the "static" implementation refer to a DBContext that would be instanciated in the INIT, and move all the transaction aware and db connection management inside the DBContext, I am sure we can expect to find a way to keep the existing API as is. The Automapper did a similar move, that is moving everything from static implementation to (ie singleton) to transient injectable objects. And it solved quite a bit of headaches. With multi-threading and high volume apps, it is safer to "Create-Use-Dispose" objects. Keeping the existing static is important, but I am sure that following the current trend, Create-Use-Dispose, it will keep your great library around longer and will encourage it's usage. |
yeah, i'm leaning towards that too... just need to find the time to do a rewrite... |
Here is an example of static being not so great.... DB.InitAsync(_dbName, settings) behing called at the startup. Later
Then the susbscribe is not considered because the internal
And since the logger is diposable..... then we get ObjectDisposedException Is there a way to expose the skipNetworkPing and for a replace in the DB.dbs ? Maybe add it as an optional parameter to InitAsync ? Or a way to manually clear/remove the entry ? For the moment I used this in conjuction with Dependency Injection.... not great... but it works if I need the "logging" to work.
|
hopefully #125 will sort out those headaches... |
That's my solution for now, multitenancy using the same DB, Autofac Multinancy Custom DBContext: public class ClientDbContext : DBContext
{
public string TenantId { get; }
public ClientDbContext(string tenantId)
{
TenantId = tenantId;
SetGlobalFilterForBaseClass<ClientEntity>(
filter: b => b.TenantID == tenantId,
prepend: true);
SetGlobalFilterForBaseClass<ClientFile>(
filter: b => b.TenantID == tenantId,
prepend: true);
}
protected override Action<T> OnBeforeSave<T>()
{
Action<ClientEntity> action = f => { f.TenantID = TenantId; };
return action as Action<T>;
}
protected override Action<UpdateBase<T>> OnBeforeUpdate<T>()
{
Action<UpdateBase<ClientEntity>> action = update =>
{
update.AddModification(f => f.TenantID, TenantId);
};
return action as Action<UpdateBase<T>>;
}
} Tenant Identification Strategy: public class SchoolTenantIdentificationStrategy : ITenantIdentificationStrategy
{
private IHttpContextAccessor _accessor;
public SchoolTenantIdentificationStrategy(IHttpContextAccessor accessor)
{
_accessor = accessor;
}
public bool TryIdentifyTenant(out object tenantId)
{
var clientId = _accessor.HttpContext?.User.Claims.First(x => x.Type == "TenantId").Value;
tenantId = clientId;
if (clientId is null)
return false;
return true;
}
} So far the performance is great |
As per the documentation at https://mongodb-entities.com/wiki/DB-Instances.html, I am trying to create DBContext per request based on TenantId (identified via a Middleware). The DBContext is returned via below method per request
I inherited DBContext and created MongoDbContext since I need to add more features to it in future. Seems like I can't use what's mentioned in https://mongodb-entities.com/wiki/Multiple-Databases.html for my purpose. Am I correct to assume that the library cannot switch databases per request as above? |
@mkgn yeah db per tenant is not supported with the current state of the library. PRs are welcome from the community if it can be done in a way that's not too difficult to migrate existing code bases. |
Would it be possible with the DB.DatabaseFor<entity>(dbName) to also have support for a "tenant".
This would enable the storage of the same entity type, but in different database depending on the request.
For example I would like to implement this and use MongoDB.Entities (and not MongoFramework)
Finbuckle.Multitenant
Maybe something like: DB.DatabaseFor<entity, ITenantInfo>(dbName)
or something implemented via the DbContext.
The text was updated successfully, but these errors were encountered: