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.

Sign up for GitHub

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

Jump to bottom

Avoid logging every pull query in INFO level #3672

Closed
vinothchandar opened this issue Oct 25, 2019 · 2 comments · Fixed by #3820
Closed

Avoid logging every pull query in INFO level #3672

vinothchandar opened this issue Oct 25, 2019 · 2 comments · Fixed by #3820
Assignees
@vpapavas
Projects
Pull Queries

Comments

@vinothchandar
Copy link
Contributor

The log statement in KsqlResource

 public Response handleKsqlStatements(
      @Context final ServiceContext serviceContext,
      final KsqlRequest request
  ) {
     LOG.info("Received: " + request);

may be ok for non-pull queries. but for pull queries, it will keep flooding the logs since they will be issued at much higher rate
@vinothchandar vinothchandar added this to To do in Pull Queries via automation Oct 28, 2019
@vinothchandar vinothchandar moved this from To do to In progress in Pull Queries Nov 5, 2019
@vpapavas vpapavas self-assigned this Nov 5, 2019
@vpapavas vpapavas mentioned this issue Nov 5, 2019
Closed
2 tasks
@big-andy-coates
Copy link
Contributor

This can be fixed by moving pull queries onto the /query endpoint

@vinothchandar
Copy link
Contributor Author

@big-andy-coates do you expect that to happen in the next week?

big-andy-coates added a commit to big-andy-coates/ksql that referenced this issue Nov 12, 2019
@big-andy-coates
fix: pull queries available on /query rest & ws endpoint
8e7cba9 
fixes: confluentinc#3672 by providing alternative way of issuing pull queries that does NOT log

Makes pull queries available on the `/query` RESTful and Websocket endpoints, in the same way that push queries are.

Note: this change does not _remove_ pull query support from the `/ksql` endpoint, nor does it switch the CLI over to use
the `/query` endpoint. The CLI continues to use the `/ksql` endpoint for pull queries.

Push and pull queries to the `/query` rest endpoint now return the schema of the rows in the first message.
This is required as the 'DESCRIBE' that CLI was previously running to get column headers doesn't work for pull queries yet. (Known issue: confluentinc#3495).
This is similar to the pattern used by the websocket endpoint, which also sends the schema in the first message.

In addition, I've hidden null fields and added a 'header' row to return the schema of the data. The output now looks like:

```json
[{"header":{"queryId":"someId","schema":"`USERID` STRING, `PAGEID` STRING, `VIEWTIME` BIGINT, `ROWKEY` STRING"}},
{"row":{"columns":["USER_1","PAGE_1",1,"1"]}},
{"row":{"columns":["USER_2","PAGE_2",2,"2"]}},
{"finalMessage":"Limit Reached"}]"
```

BREAKING CHANGE: the response from the RESTful API for push queries has changed: it now returns a line with the schema and query id in a `header` field and null fields are not included in the payload.

The CLI is backwards compatible with older versions of the server, though it won't output column headings from older versions.
@big-andy-coates big-andy-coates mentioned this issue Nov 12, 2019
Merged
6 tasks
vpapavas pushed a commit to vpapavas/ksql that referenced this issue Nov 14, 2019
@big-andy-coates @vpapavas
fix: pull queries available on /query rest & ws endpoint
f599831 
fixes: confluentinc#3672 by providing alternative way of issuing pull queries that does NOT log

Makes pull queries available on the `/query` RESTful and Websocket endpoints, in the same way that push queries are.

Note: this change does not _remove_ pull query support from the `/ksql` endpoint, nor does it switch the CLI over to use
the `/query` endpoint. The CLI continues to use the `/ksql` endpoint for pull queries.

Push and pull queries to the `/query` rest endpoint now return the schema of the rows in the first message.
This is required as the 'DESCRIBE' that CLI was previously running to get column headers doesn't work for pull queries yet. (Known issue: confluentinc#3495).
This is similar to the pattern used by the websocket endpoint, which also sends the schema in the first message.

In addition, I've hidden null fields and added a 'header' row to return the schema of the data. The output now looks like:

```json
[{"header":{"queryId":"someId","schema":"`USERID` STRING, `PAGEID` STRING, `VIEWTIME` BIGINT, `ROWKEY` STRING"}},
{"row":{"columns":["USER_1","PAGE_1",1,"1"]}},
{"row":{"columns":["USER_2","PAGE_2",2,"2"]}},
{"finalMessage":"Limit Reached"}]"
```

BREAKING CHANGE: the response from the RESTful API for push queries has changed: it now returns a line with the schema and query id in a `header` field and null fields are not included in the payload.

The CLI is backwards compatible with older versions of the server, though it won't output column headings from older versions.
Pull Queries automation moved this from In progress to Done Nov 14, 2019
big-andy-coates added a commit that referenced this issue Nov 14, 2019
@big-andy-coates
fix: pull queries available on /query rest & ws endpoint (#3820)
e2321f5 
* fix: pull queries available on `/query` rest & ws endpoint

fixes: #3672 by providing alternative way of issuing pull queries that does NOT log

Makes pull queries available on the `/query` RESTful and Websocket endpoints, in the same way that push queries are.

Note: this change does not _remove_ pull query support from the `/ksql` endpoint, nor does it switch the CLI over to use
the `/query` endpoint. The CLI continues to use the `/ksql` endpoint for pull queries.

Push and pull queries to the `/query` rest endpoint now return the schema of the rows in the first message.
This is required as the 'DESCRIBE' that CLI was previously running to get column headers doesn't work for pull queries yet. (Known issue: #3495).
This is similar to the pattern used by the websocket endpoint, which also sends the schema in the first message.

In addition, I've hidden null fields and added a 'header' row to return the schema of the data. The output now looks like:

```json
[{"header":{"queryId":"someId","schema":"`USERID` STRING, `PAGEID` STRING, `VIEWTIME` BIGINT, `ROWKEY` STRING"}},
{"row":{"columns":["USER_1","PAGE_1",1,"1"]}},
{"row":{"columns":["USER_2","PAGE_2",2,"2"]}},
{"finalMessage":"Limit Reached"}]"
```

BREAKING CHANGE: the response from the RESTful API for push queries has changed: it now returns a line with the schema and query id in a `header` field and null fields are not included in the payload.

The CLI is backwards compatible with older versions of the server, though it won't output column headings from older versions.
big-andy-coates added a commit that referenced this issue Nov 14, 2019
@big-andy-coates
fix: pull queries available on /query rest & ws endpoint (#3820)
9a47eaf 
* fix: pull queries available on `/query` rest & ws endpoint

fixes: #3672 by providing alternative way of issuing pull queries that does NOT log

Makes pull queries available on the `/query` RESTful and Websocket endpoints, in the same way that push queries are.

Note: this change does not _remove_ pull query support from the `/ksql` endpoint, nor does it switch the CLI over to use
the `/query` endpoint. The CLI continues to use the `/ksql` endpoint for pull queries.

Push and pull queries to the `/query` rest endpoint now return the schema of the rows in the first message.
This is required as the 'DESCRIBE' that CLI was previously running to get column headers doesn't work for pull queries yet. (Known issue: #3495).
This is similar to the pattern used by the websocket endpoint, which also sends the schema in the first message.

In addition, I've hidden null fields and added a 'header' row to return the schema of the data. The output now looks like:

```json
[{"header":{"queryId":"someId","schema":"`USERID` STRING, `PAGEID` STRING, `VIEWTIME` BIGINT, `ROWKEY` STRING"}},
{"row":{"columns":["USER_1","PAGE_1",1,"1"]}},
{"row":{"columns":["USER_2","PAGE_2",2,"2"]}},
{"finalMessage":"Limit Reached"}]"
```

BREAKING CHANGE: the response from the RESTful API for push queries has changed: it now returns a line with the schema and query id in a `header` field and null fields are not included in the payload.

The CLI is backwards compatible with older versions of the server, though it won't output column headings from older versions.

(cherry picked from commit e2321f5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vpapavas vpapavas

Labels
None yet
Projects
Pull Queries
  
Done
Milestone
No milestone
3 participants
@vinothchandar @vpapavas @big-andy-coates