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

PHOENIX-3547 Supporting more number of indices per table. #324

Closed
wants to merge 17 commits into from

Conversation

m2je
Copy link

@m2je m2je commented Aug 14, 2018

Currently the number of indices per Phoenix table is bound to maximum of 65535 (java.lang.Short) which is a limitation for applications requiring to have unlimited number of indices.
This change will consider any new table created in Phoenix to support view index ids to be in the range of -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 65535 of indices.
A new int column (VIEW_INDEX_ID_DATA_TYPE TINYINT) is added to SYSTEM.CATALOG to specify each Phoenix table's viewIndex data type.
On each new Phoenix table creation the value for VIEW_INDEX_ID_DATA_TYPE will be set to Long while this value would be Short for any existing table.
According to Protobuf documentation https://developers.google.com/protocol-buffers/docs/proto#updating we can change the type of viewIndexId from int32 to int64 and maintain the backward compatibility for older clients. We did a manual verification for this scenario by
creating a view with the old client and verify the new client is able to connect and read/write to the view after changing the viewIndexId type and adding the viewIndexType.

Mahdi Salarkia and others added 15 commits July 27, 2018 22:27
Currently the number of indices per Phoenix table is bound to maximum of 65535 (java.lang.Short) which is a limitation for applications requiring to have unlimited number of indices.
This change will consider any new table created in Phoenix to support view index ids to be in the range of -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 65535 of indices.
A new boolean column (USE_LONG_VIEW_INDEX BOOLEAN DEFAULT FALSE) is added to SYSTEM.CATALOG to specify each Phoenix table's support for large number of indices.
On each new Phoenix table creation the value for USE_LONG_VIEW_INDEX will be set to `true` while this value would be false for any existing table.
Currently the number of indices per Phoenix table is bound to maximum of 65535 (java.lang.Short) which is a limitation for applications requiring to have unlimited number of indices.
This change will consider any new table created in Phoenix to support view index ids to be in the range of -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 65535 of indices.
A new boolean column (USE_LONG_VIEW_INDEX BOOLEAN DEFAULT FALSE) is added to SYSTEM.CATALOG to specify each Phoenix table's support for large number of indices.
On each new Phoenix table creation the value for USE_LONG_VIEW_INDEX will be set to `true` while this value would be false for any existing table.
Closes apache#318

Signed-off-by: Josh Elser <elserj@apache.org>
Currently the number of indices per Phoenix table is bound to maximum of 65535 (java.lang.Short) which is a limitation for applications requiring to have unlimited number of indices.
This change will consider any new table created in Phoenix to support view index ids to be in the range of -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 65535 of indices.
A new int column (VIEW_INDEX_ID_DATA_TYPE TINYINT) is added to SYSTEM.CATALOG to specify each Phoenix table's vewIndex data type.
On each new Phoenix table creation the value for VIEW_INDEX_ID_DATA_TYPE will be set to `Long` while this value would be `Short` for any existing table.
Currently the number of indices per Phoenix table is bound to maximum of 65535 (java.lang.Short) which is a limitation for applications requiring to have unlimited number of indices.
This change will consider any new table created in Phoenix to support view index ids to be in the range of -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 65535 of indices.
A new int column (VIEW_INDEX_ID_DATA_TYPE TINYINT) is added to SYSTEM.CATALOG to specify each Phoenix table's vewIndex data type.
On each new Phoenix table creation the value for VIEW_INDEX_ID_DATA_TYPE will be set to `Long` while this value would be `Short` for any existing table.
@m2je
Copy link
Author

m2je commented Aug 15, 2018

@twdsilva Tested the viewIndexId change from int32 to int64 by creating a view with Phoenix/hbase driver 4.13.2 and then query/insert,... using the patched driver and it worked properly.

@m2je m2je closed this Aug 15, 2018
@m2je m2je reopened this Aug 15, 2018
@twdsilva
Copy link
Contributor

@m2je Just to confirm the old client was still able to use the view that it had created right ?

Copy link
Contributor

@twdsilva twdsilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m2je
Its looking pretty good, I requested just some minor changes.
Can you also create a patch file and upload it to the JIRA and click submit patch so that we can get a jenkins run?

@@ -382,6 +389,7 @@
private static final int DISABLE_WAL_INDEX = TABLE_KV_COLUMNS.indexOf(DISABLE_WAL_KV);
private static final int MULTI_TENANT_INDEX = TABLE_KV_COLUMNS.indexOf(MULTI_TENANT_KV);
private static final int VIEW_TYPE_INDEX = TABLE_KV_COLUMNS.indexOf(VIEW_TYPE_KV);
private static final int VIEW_INDEX_ID_DATA_TYPE = TABLE_KV_COLUMNS.indexOf(VIEW_INDEX_ID_DATA_TYPE_BYTES_KV);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rename to VIEW_INDEX_ID_DATA_TYPE_INDEX

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

private PDataType getViewIndexType(Cell[] tableKeyValues) {
Cell dataTypeKv = tableKeyValues[VIEW_INDEX_ID_DATA_TYPE];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need a null check here similar to the getViewIndexId. If the view index was created by an old client it won't have the VIEW_INDEX_ID_DATA_TYPE and you can assume its a short.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Mahdi Salarkia added 2 commits August 19, 2018 17:25
Currently the number of indices per Phoenix table is bound to maximum of 65535 (java.lang.Short) which is a limitation for applications requiring to have unlimited number of indices.
This change will consider any new table created in Phoenix to support view index ids to be in the range of -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 65535 of indices.
A new int column (VIEW_INDEX_ID_DATA_TYPE TINYINT) is added to SYSTEM.CATALOG to specify each Phoenix table's viewIndex data type.
On each new Phoenix table creation the value for VIEW_INDEX_ID_DATA_TYPE will be set to `Long` while this value would be `Short` for any existing table.
According to Protobuf documentation https://developers.google.com/protocol-buffers/docs/proto#updating we can change the type of viewIndexId from int32 to int64 and maintain the backward compatibility for older clients. We did a manual verification for this scenario by creating a view with the old client and verify the new client is able to connect and read/write to the view after changing the viewIndexId type and adding the viewIndexType.
… of 65535 (java.lang.Short) which is a limitation for applications requiring to have unlimited number of indices.

This change will consider any new table created in Phoenix to support view index ids to be in the range of -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 (java.lang.Long) which is undoubtably big enough to cover this requirement.
Any existing Phoenix table will still continue to support only maximum of 65535 of indices.
A new int column (VIEW_INDEX_ID_DATA_TYPE TINYINT) is added to SYSTEM.CATALOG to specify each Phoenix table's viewIndex data type.
On each new Phoenix table creation the value for VIEW_INDEX_ID_DATA_TYPE will be set to `Long` while this value would be `Short` for any existing table.
According to Protobuf documentation https://developers.google.com/protocol-buffers/docs/proto#updating we can change the type of viewIndexId from int32 to int64 and maintain the backward compatibility for older clients. We did a manual verification for this scenario by creating a view with the old client and verify the new client is able to connect and read/write to the view after changing the viewIndexId type and adding the viewIndexType.
@m2je
Copy link
Author

m2je commented Aug 20, 2018

@twdsilva Yes, the old client was able to interact with no problem.

@twdsilva
Copy link
Contributor

@m2je This PR has a bunch of many commits, can you please rebase and squash all your commits into a single commit. After that you can generate a .patch file and attach it to the JIRA and the click on the "Submit Patch" button so that we can get a test run.

@m2je
Copy link
Author

m2je commented Aug 21, 2018

👍 Closed this and follow up in #334

@m2je m2je closed this Aug 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants