Maxwell-Guo commented on code in PR #2879:
URL: https://github.com/apache/cassandra/pull/2879#discussion_r1413424748
##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormatPartitionWriter.java:
##########
@@ -70,7 +70,12 @@ public class BigFormatPartitionWriter extends
SortedTablePartitionWriter
Version version,
ISerializer<IndexInfo> indexInfoSerializer)
{
- this(header, writer, version, indexInfoSerializer,
DatabaseDescriptor.getColumnIndexCacheSize(),
DatabaseDescriptor.getColumnIndexSize(DEFAULT_GRANULARITY));
+ this(header,
+ writer,
+ version,
+ indexInfoSerializer,
+
(int)version.getSSTableFormatValue(Option.COLUMN_INDEX_CACHE_SIZE),
Review Comment:
what about add interfaces(getRowIndexCacheSize、getRowIndexGranularity and
other methods) in`SSTableFormat` , and for BIG and BTI, We can implement
supported options, and throw exceptions if they are not supported.
As you have said `This will likely require changes to
DatabaseDescriptor.setXXX methods to make sure they update the format instances
rather than the database descriptor ones.` , we may parse the value from the
[option
maps](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/io/sstable/format/AbstractSSTableFormat.java#L27),
so I think we should add some method called
`DatabaseDescriptor.setFormatOption(formatName, optionName, optionValue)` so
that we can modify the options map in different instance
Besides, I still want to consult. I want to put all common options in
SSTableFormat.java as
[Components](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/io/sstable/format/SSTableFormat.java#L155)
has done. BIG and BTI can has their own options .
##########
src/java/org/apache/cassandra/io/sstable/format/big/BigFormatPartitionWriter.java:
##########
@@ -70,7 +70,12 @@ public class BigFormatPartitionWriter extends
SortedTablePartitionWriter
Version version,
ISerializer<IndexInfo> indexInfoSerializer)
{
- this(header, writer, version, indexInfoSerializer,
DatabaseDescriptor.getColumnIndexCacheSize(),
DatabaseDescriptor.getColumnIndexSize(DEFAULT_GRANULARITY));
+ this(header,
+ writer,
+ version,
+ indexInfoSerializer,
+
(int)version.getSSTableFormatValue(Option.COLUMN_INDEX_CACHE_SIZE),
Review Comment:
what about add interfaces(getRowIndexCacheSize、getRowIndexGranularity and
other methods) in`SSTableFormat` , and for BIG and BTI, We can implement
supported options, and throw exceptions if they are not supported.
As you have said `This will likely require changes to
DatabaseDescriptor.setXXX methods to make sure they update the format instances
rather than the database descriptor ones.` , we may parse the value from the
[option
maps](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/io/sstable/format/AbstractSSTableFormat.java#L27),
so I think we should add some method called
`DatabaseDescriptor.setFormatOption(formatName, optionName, optionValue)` so
that we can modify the options map in different instance
Besides, I still want to consult. I want to put all common options in
SSTableFormat.java as
[Components](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/io/sstable/format/SSTableFormat.java#L155)
has done. BIG and BTI can has their own options .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
