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

A more elegant implement for temporary table #26952

Closed
7 tasks done
lcwangchao opened this issue Aug 6, 2021 · 2 comments
Closed
7 tasks done

A more elegant implement for temporary table #26952

lcwangchao opened this issue Aug 6, 2021 · 2 comments
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@lcwangchao
Copy link
Collaborator

lcwangchao commented Aug 6, 2021

Enhancement

Temporary tables do not read/write TiKV. For global temporary table, it has an empty snapshot data and for local temporary table, it keeps snapshot data in session. Currently, snapshot does not support reading from memory, so when reading a temporary table's snapshot record, we need to do some tricks.

For example in point get, when reading snapshot, just get the record from session (nil record if it is a global temporary table).

tidb/executor/point_get.go

Lines 405 to 429 in 300f159

// Global temporary table is always empty, so no need to send the request.
if e.tblInfo.TempTableType == model.TempTableGlobal {
return nil, nil
}
// Local temporary table always get snapshot value from session
if e.tblInfo.TempTableType == model.TempTableLocal {
return e.ctx.GetSessionVars().TemporaryTableSnapshotReader(e.tblInfo).Get(ctx, key)
}
lock := e.tblInfo.Lock
if lock != nil && (lock.Tp == model.TableLockRead || lock.Tp == model.TableLockReadOnly) {
if e.ctx.GetSessionVars().EnablePointGetCache {
cacheDB := e.ctx.GetStore().GetMemCache()
val, err = cacheDB.UnionGet(ctx, e.tblInfo.ID, e.snapshot, key)
if err != nil {
return nil, err
}
return val, nil
}
}
// if not read lock or table was unlock then snapshot get
return e.snapshot.Get(ctx, key)
}

For batch_point_get, we make a custom snapshot temporaryTableSnapshot implements kv.Snapshot to read temporary session data.

// Avoid network requests for the temporary table.
if e.tblInfo.TempTableType == model.TempTableGlobal {
snapshot = temporaryTableSnapshot{snapshot, nil}
} else if e.tblInfo.TempTableType == model.TempTableLocal {
snapshot = temporaryTableSnapshot{snapshot, e.ctx.GetSessionVars().TemporaryTableData}
}
var batchGetter kv.BatchGetter = snapshot
if txn.Valid() {
lock := e.tblInfo.Lock
if e.lock {
batchGetter = driver.NewBufferBatchGetter(txn.GetMemBuffer(), &PessimisticLockCacheGetter{txnCtx: txnCtx}, snapshot)
} else if lock != nil && (lock.Tp == model.TableLockRead || lock.Tp == model.TableLockReadOnly) && e.ctx.GetSessionVars().EnablePointGetCache {
batchGetter = newCacheBatchGetter(e.ctx, e.tblInfo.ID, snapshot)
} else {
batchGetter = driver.NewBufferBatchGetter(txn.GetMemBuffer(), nil, snapshot)
}
}
e.snapshot = snapshot
e.batchGetter = batchGetter
return nil
}

The above examples are not elegant, that is because we must add special codes in EVERY places where we need to access snapshot data. If we forget to do this, bug occurs.

A better practice is to make the kv.Snapshot more generic. That is, when the key is belongs to a temporary table, it read it from the session, otherwise from TiKV. After that, we can delete a lot of codes like if tblInfo.TempTableType != model.TempTableNone and to make a unified snapshot access for both temporary and normal table.

One difficulty to achieve it is that Snapshot object does not have the context of the table currently reading. So it can not infer whether to get the data from memory or not. In fact, snapshot does not have any concept for table, it is a more underlaying utility to access bytes key and values. We need to introduce some different abstractions to snapshot to make it.

Steps:

Below pull requests are outdated and not used by new design

@lcwangchao lcwangchao added the type/enhancement The issue or PR belongs to an enhancement. label Aug 6, 2021
@lcwangchao lcwangchao self-assigned this Aug 6, 2021
@lcwangchao
Copy link
Collaborator Author

lcwangchao commented Aug 6, 2021

I posted a draft here: tikv/client-go#262 to make KVSnapshot able to read data from memory. It introduces a methods SetSortedCustomKeyRetrievers to set the ranges for custom reading.

I posted another draft in tidb: #26948 to use above Snapshot to access temporary table snapshot data.

@djshow832
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants