-
Notifications
You must be signed in to change notification settings - Fork 19
Lack of schema refresh after new field in common table #1461
Lack of schema refresh after new field in common table #1461
Conversation
jakozaur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test should be faster.
| assert.Equal(t, http.StatusOK, resp.StatusCode) | ||
|
|
||
| // wait for internal processing, especially for the periodic task that updates the schema | ||
| time.Sleep(60 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this is very tricky to add. Slow IT are hard to debug and takes a lot to some engineering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I know.
I will add an endpoint to reload tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the test. Added endpoint. And fixed "eventually consistent" index that keeps a list of virtual tables.
|
/run-it |
1 similar comment
|
/run-it |
jakozaur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be merged, but move it to admin interface.
| func (a *OnlyCommonTableTestcase) testAlterVirtualTable(ctx context.Context, t *testing.T) { | ||
|
|
||
| reloadTables := func() { | ||
| resp, body := a.RequestToQuesma(ctx, t, "POST", "/_quesma/reload-tables", nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should put that method under our admin port, not on ElasticSearch frontend.
As we are rushing with release fine to do in next PR.
| return &quesma_api.Result{Body: string(body), StatusCode: http.StatusOK, GenericResult: body}, nil | ||
| }) | ||
|
|
||
| router.Register(routes.QuesmaReloadTablsPath, method("POST"), func(ctx context.Context, req *quesma_api.Request, _ http.ResponseWriter) (*quesma_api.Result, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, please move this to admin interface.
Quesma relies on
attributescolumns when it calculates the schema change on ingest. Table discovery didn't add these fields to virtual tables. Therefore, the schema of the index stored in thecommon tablebecomes immutable within 60 seconds.